From 8a1d96666109a7236ff7d3c7eba96783aed309ab Mon Sep 17 00:00:00 2001 From: Romain Beauxis Date: Sat, 18 Jan 2025 10:49:05 -0600 Subject: [PATCH] Cleanup frame metadata/track marks API Enhance ffmpeg IO metadata handling. --- src/core/io/ffmpeg_io.ml | 47 ++++++++++++++++---- src/core/operators/add.ml | 2 +- src/core/operators/insert_metadata.ml | 3 +- src/core/operators/time_warp.ml | 2 +- src/core/operators/track/merge_metadata.ml | 25 ++++------- src/core/outputs/output.ml | 4 +- src/core/source.ml | 6 +-- src/core/sources/blank.ml | 4 +- src/core/sources/generated.ml | 2 +- src/core/sources/request_dynamic.ml | 4 +- src/core/sources/synthesized.ml | 2 +- src/core/stream/frame.ml | 50 +++++++--------------- src/core/stream/frame.mli | 16 +++---- 13 files changed, 85 insertions(+), 82 deletions(-) diff --git a/src/core/io/ffmpeg_io.ml b/src/core/io/ffmpeg_io.ml index 736b2649a7..a93290a7c8 100644 --- a/src/core/io/ffmpeg_io.ml +++ b/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 diff --git a/src/core/operators/add.ml b/src/core/operators/add.ml index 5cff41ac35..baca2b6f66 100644 --- a/src/core/operators/add.ml +++ b/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. diff --git a/src/core/operators/insert_metadata.ml b/src/core/operators/insert_metadata.ml index 72fcecd3f2..ef0c4c55be 100644 --- a/src/core/operators/insert_metadata.ml +++ b/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 diff --git a/src/core/operators/time_warp.ml b/src/core/operators/time_warp.ml index acdaa46879..8953af0df1 100644 --- a/src/core/operators/time_warp.ml +++ b/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 diff --git a/src/core/operators/track/merge_metadata.ml b/src/core/operators/track/merge_metadata.ml index 3837fb5eee..37e060bfe3 100644 --- a/src/core/operators/track/merge_metadata.ml +++ b/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 _ = diff --git a/src/core/outputs/output.ml b/src/core/outputs/output.ml index a2622a88c1..a17707069a 100644 --- a/src/core/outputs/output.ml +++ b/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; diff --git a/src/core/source.ml b/src/core/source.ml index 7f5852000b..f90332b0ac 100644 --- a/src/core/source.ml +++ b/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 diff --git a/src/core/sources/blank.ml b/src/core/sources/blank.ml index 59e8a7a859..b7a7dc4f67 100644 --- a/src/core/sources/blank.ml +++ b/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) diff --git a/src/core/sources/generated.ml b/src/core/sources/generated.ml index b676697c51..25b9942b34 100644 --- a/src/core/sources/generated.ml +++ b/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 ( diff --git a/src/core/sources/request_dynamic.ml b/src/core/sources/request_dynamic.ml index 00a29985d8..72211466e5 100644 --- a/src/core/sources/request_dynamic.ml +++ b/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 diff --git a/src/core/sources/synthesized.ml b/src/core/sources/synthesized.ml index 2fba02169a..9e3b83914b 100644 --- a/src/core/sources/synthesized.ml +++ b/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 diff --git a/src/core/stream/frame.ml b/src/core/stream/frame.ml index c4b40fc5ed..de63ef149e 100644 --- a/src/core/stream/frame.ml +++ b/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) diff --git a/src/core/stream/frame.mli b/src/core/stream/frame.mli index 3ba5659c75..f496c60bb5 100644 --- a/src/core/stream/frame.mli +++ b/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} *)