Cleanup frame metadata/track marks API

Enhance ffmpeg IO metadata handling.

src/core/io/ffmpeg_io.ml

@@ -241,6 +241,12 @@ class input ?(name = "input.ffmpeg") ~autostart ~self_sync ~poll_delay ~debug
         | `Connected (_, c) -> c
         | _ -> raise Not_connected
 
+    (* `icy-*` metadata seem to arrive at random position with the
+       http input. We store them here and add them to each new non-icy
+       metadata. *)
+    val mutable icy_metadata = Frame.Metadata.empty
+    val mutable icy_metadata_sent = false
+
     method private generate_frame =
       let size = Lazy.force Frame.size in
       try
@@ -255,17 +261,42 @@ class input ?(name = "input.ffmpeg") ~autostart ~self_sync ~poll_delay ~debug
           Generator.add_metadata self#buffer (Frame.Metadata.from_list meta);
           if new_track_on_metadata then Generator.add_track_mark self#buffer);
         let frame = Generator.slice self#buffer size in
-        (* Metadata can be added by the decoder and the demuxer so we filter at the frame level. *)
-        let metadata =
+        (* Make sure only one track mark is added per frame. *)
+        let frame =
+          match Frame.track_marks frame with
+            | m :: _ -> Frame.set_track_mark frame m
+            | _ -> frame
+        in
+        (* Metadata can be added by the decoder and the demuxer so we filter at the frame level.
+           Also, with the new media API, it's better tot only have one metadata per frame. *)
+        let p, metadata =
           List.fold_left
-            (fun metadata (p, m) ->
-              let m = metadata_filter m in
-              if 0 < Frame.Metadata.cardinal m then (p, m) :: metadata
-              else metadata)
-            []
+            (fun (p, m) (p', m') -> (min p p', Frame.Metadata.append m m'))
+            (max_int, Frame.Metadata.empty)
             (Frame.get_all_metadata frame)
         in
-        Frame.add_all_metadata frame metadata
+        let metadata =
+          Frame.Metadata.fold
+            (fun lbl v metadata ->
+              if String.starts_with ~prefix:"icy-" lbl then (
+                icy_metadata <- Frame.Metadata.add lbl v icy_metadata;
+                metadata)
+              else Frame.Metadata.add lbl v metadata)
+            metadata Frame.Metadata.empty
+        in
+        let metadata =
+          match
+            ( Frame.Metadata.is_empty metadata,
+              Frame.Metadata.is_empty icy_metadata )
+          with
+            | true, false when not icy_metadata_sent -> icy_metadata
+            | false, _ -> Frame.Metadata.append icy_metadata metadata
+            | _ -> Frame.Metadata.empty
+        in
+        let metadata = metadata_filter metadata in
+        icy_metadata_sent <- not (Frame.Metadata.is_empty icy_metadata);
+        if Frame.Metadata.is_empty metadata then Frame.clear_metadata frame
+        else Frame.set_all_metadata frame [(p, metadata)]
       with exn ->
         let bt = Printexc.get_raw_backtrace () in
         Utils.log_exception ~log:self#log

src/core/operators/add.ml

@@ -107,7 +107,7 @@ class virtual base ~name tracks =
               Content.Metadata.get_data
                 (Frame.Fields.find Frame.Fields.metadata source#get_frame)
             in
-            Frame.add_all_metadata buf metadata
+            Frame.set_all_metadata buf metadata
   end
 
 (** Add/mix several sources together.

src/core/operators/insert_metadata.ml

@@ -50,8 +50,7 @@ class insert_metadata source =
                 m
             in
             let buf = Frame.add_metadata buf 0 m in
-            if new_track then Frame.(add_track_mark (drop_track_marks buf)) 0
-            else buf
+            if new_track then Frame.set_track_mark buf 0 else buf
         | None -> buf
   end
 

src/core/operators/time_warp.ml

@@ -343,7 +343,7 @@ module AdaptativeBuffer = struct
             in
             let frame =
               match Frame.track_marks content with
-                | p :: _ -> Frame.add_track_mark frame (unscale p)
+                | p :: _ -> Frame.set_track_mark frame (unscale p)
                 | _ -> frame
             in
 

src/core/operators/track/merge_metadata.ml

@@ -44,22 +44,15 @@ class merge_metadata tracks =
       match self#ready_sources with
         | [] -> assert false
         | s :: rest ->
-            List.fold_left
-              (fun frame source ->
-                let l = Frame.get_all_metadata source#get_frame in
-                let l =
-                  List.fold_left
-                    (fun l (pos, m) ->
-                      ( pos,
-                        Frame.Metadata.append
-                          (Option.value ~default:Frame.Metadata.empty
-                             (Frame.get_metadata frame pos))
-                          m )
-                      :: l)
-                    [] l
-                in
-                Frame.add_all_metadata frame l)
-              s#get_frame rest
+            let frame = s#get_frame in
+            let metadata =
+              Frame.get_all_metadata frame
+              :: List.map
+                   (fun source -> Frame.get_all_metadata source#get_frame)
+                   rest
+            in
+            let metadata = List.flatten metadata in
+            Frame.set_all_metadata frame metadata
   end
 
 let _ =

src/core/outputs/output.ml

@@ -166,8 +166,8 @@ class virtual output ~output_kind ?clock ?(name = "") ~infallible
     method private skip = skip <- true
 
     method private generate_frame =
-      Frame.map_metadata source#get_frame (fun (pos, m) ->
-          Some (pos, self#add_on_air m))
+      Frame.map_metadata source#get_frame
+        (List.map (fun (pos, m) -> (pos, self#add_on_air m)))
 
     method output =
       if self#is_ready && state = `Idle then start_stop#transition_to `Started;

src/core/source.ml

@@ -435,7 +435,7 @@ class virtual operator ?(stack = []) ?clock ?(name = "src") sources =
             empty_frame <- Some f;
             f
 
-    method end_of_track = Frame.add_track_mark self#empty_frame 0
+    method end_of_track = Frame.set_track_mark self#empty_frame 0
     val mutable last_metadata = None
     method last_metadata = last_metadata
     val mutable on_metadata : (Frame.metadata -> unit) List.t = []
@@ -550,7 +550,7 @@ class virtual operator ?(stack = []) ?clock ?(name = "src") sources =
               self#log#important
                 "Source created multiple tracks in a single frame! Sub-frame \
                  tracks are not supported and are merged into a single one..";
-              Frame.add_track_mark (Frame.drop_track_marks buf) p
+              Frame.set_track_mark (Frame.drop_track_marks buf) p
           | _ -> buf
       in
       let has_track_mark = track_marks <> [] in
@@ -625,7 +625,7 @@ class virtual generate_from_multiple_sources ~merge ~track_sensitive () =
       if merge () then Frame.drop_track_marks buf
       else (
         self#execute_on_track buf;
-        Frame.add_track_mark buf 0)
+        Frame.set_track_mark buf 0)
 
     method private can_reselect ~(reselect : reselect) (s : source) =
       s#is_ready

src/core/sources/blank.ml

@@ -100,14 +100,14 @@ class blank duration =
       match (Atomic.get position, self#remaining) with
         | `New_track, _ ->
             Atomic.set position (`Elapsed length);
-            Frame.add_track_mark frame 0
+            Frame.set_track_mark frame 0
         | `Elapsed d, -1 ->
             Atomic.set position (`Elapsed (d + length));
             frame
         | `Elapsed d, r ->
             if r < length then (
               Atomic.set position (`Elapsed (length - r));
-              Frame.add_track_mark frame r)
+              Frame.set_track_mark frame r)
             else (
               Atomic.set position (`Elapsed (d + length));
               frame)

src/core/sources/generated.ml

@@ -102,7 +102,7 @@ class virtual source ?(seek = false) ?(replay_meta = false) ~bufferize
             if was_buffering || add_track_mark then (
               self#log#info "Adding track mark.";
               add_track_mark <- false;
-              Frame.add_track_mark buf 0)
+              Frame.set_track_mark buf 0)
             else buf
           in
           if Generator.length self#buffer = 0 then (

src/core/sources/request_dynamic.ml

@@ -145,7 +145,7 @@ class dynamic ?(name = "request.dynamic") ~retry_delay ~available ~prefetch
             if first_fill then (
               let m = Request.metadata cur.req in
               let buf = Frame.add_metadata buf 0 m in
-              let buf = Frame.add_track_mark buf 0 in
+              let buf = Frame.set_track_mark buf 0 in
               first_fill <- false;
               buf)
             else buf
@@ -160,7 +160,7 @@ class dynamic ?(name = "request.dynamic") ~retry_delay ~available ~prefetch
           in
           if Atomic.get should_skip || Frame.is_partial buf then (
             self#end_request;
-            let buf = Frame.add_track_mark buf (Frame.position buf) in
+            let buf = Frame.set_track_mark buf (Frame.position buf) in
             if self#fetch_request then fill buf else buf)
           else buf)
         else buf

src/core/sources/synthesized.ml

@@ -70,6 +70,6 @@ class virtual source ?name ~seek duration =
       if add_track_mark then (
         add_track_mark <- false;
         remaining <- track_size;
-        Frame.add_track_mark buf 0)
+        Frame.set_track_mark buf 0)
       else buf
   end

src/core/stream/frame.ml

@@ -107,20 +107,13 @@ let track_marks frame =
 let has_track_marks frame = track_marks frame <> []
 let has_track_mark frame pos = List.mem pos (track_marks frame)
 
-let add_track_marks frame l =
+let set_track_mark frame pos =
   let old_marks = get frame Fields.track_marks in
-  let length =
-    List.fold_left
-      (fun length pos -> max pos length)
-      (Content.length old_marks) l
-  in
+  let length = max (Content.length old_marks) pos in
   let new_marks = Content.make ~length (Content.format old_marks) in
-  Content.Track_marks.set_data new_marks
-    (List.sort_uniq Stdlib.compare (l @ Content.Track_marks.get_data old_marks));
+  Content.Track_marks.set_data new_marks [pos];
   Fields.add Fields.track_marks new_marks frame
 
-let add_track_mark frame pos = add_track_marks frame [pos]
-
 let drop_track_marks frame =
   Fields.add Fields.track_marks
     (Content.make ~length:(position frame) Content_timed.Track_marks.format)
@@ -136,33 +129,20 @@ let get_all_metadata frame =
 let get_metadata b t =
   try Some (List.assoc t (get_all_metadata b)) with Not_found -> None
 
-let add_all_metadata frame l =
-  let old_metadata = get frame Fields.metadata in
+let map_metadata frame fn =
+  let metadata = get frame Fields.metadata in
+  let new_metadata = fn (Content.Metadata.get_data metadata) in
   let length =
-    List.fold_left
-      (fun length (pos, _) -> max pos length)
-      (Content.length old_metadata)
-      l
+    max (Content.length metadata)
+      (List.fold_left (fun p (p', _) -> max p p') 0 new_metadata)
   in
-  let new_metadata = Content.make ~length (Content.format old_metadata) in
-  Content.Metadata.set_data new_metadata
-    (List.sort_uniq
-       (fun (p, _) (p', _) -> Stdlib.compare p p')
-       (l @ Content.Metadata.get_data old_metadata));
-  Fields.add Fields.metadata new_metadata frame
+  let new_metadata_content = Content.make ~length (Content.format metadata) in
+  Content.Metadata.set_data new_metadata_content new_metadata;
+  Fields.add Fields.metadata new_metadata_content frame
 
-let map_metadata frame fn =
-  let metadata = get_all_metadata frame in
-  let metadata = List.filter_map fn metadata in
-  add_all_metadata frame metadata
-
-let add_metadata frame pos m = add_all_metadata frame [(pos, m)]
+let clear_metadata frame = map_metadata frame (fun _ -> [])
+let add_metadata frame pos m = map_metadata frame (fun meta -> (pos, m) :: meta)
+let set_all_metadata frame m' = map_metadata frame (fun _ -> m')
 
 let free_metadata frame pos =
-  let metadata = get frame Fields.metadata in
-  let new_metadata =
-    Content.make ~length:(Content.length metadata) (Content.format metadata)
-  in
-  Content.Metadata.set_data new_metadata
-    (List.filter (fun (p, _) -> p <> pos) (Content.Metadata.get_data metadata));
-  Fields.add Fields.metadata new_metadata frame
+  map_metadata frame (fun meta -> List.filter (fun (p, _) -> p <> pos) meta)

src/core/stream/frame.mli

@@ -154,11 +154,8 @@ val is_partial : t -> bool
 (** List of track marks in a frame. *)
 val track_marks : t -> int list
 
-(** Add a track mark to a frame *)
-val add_track_mark : t -> int -> t
-
-(** Add a multiple track marks to a frame *)
-val add_track_marks : t -> int list -> t
+(** Set a unique track mark to a frame *)
+val set_track_mark : t -> int -> t
 
 (** [true] is frame has a track mark. *)
 val has_track_marks : t -> bool
@@ -185,11 +182,14 @@ val get_metadata : t -> int -> metadata option
 (** Retrieve all metadata. *)
 val get_all_metadata : t -> (int * metadata) list
 
-(** Attach multiple metadata to a frame. *)
-val add_all_metadata : t -> (int * metadata) list -> t
+(** Set all metadata on a frame. *)
+val set_all_metadata : t -> (int * metadata) list -> t
 
 (** Map a function over the frame's metadata. *)
-val map_metadata : t -> (int * metadata -> (int * metadata) option) -> t
+val map_metadata : t -> ((int * metadata) list -> (int * metadata) list) -> t
+
+(** Remove all metadata from the frame. *)
+val clear_metadata : t -> t
 
 (** {2 Content operations} *)