From 554f80b43b9e906306795f8bfcb55f7ed35e13c0 Mon Sep 17 00:00:00 2001 From: Romain Beauxis Date: Fri, 28 Jul 2023 11:45:20 -0500 Subject: [PATCH] Make encoder header more robust by using a callback. Fixes: #3265 --- src/core/encoder/encoder.ml | 6 ++---- src/core/encoder/encoder.mli | 5 +---- src/core/encoder/encoders/avi_encoder.ml | 2 +- src/core/encoder/encoders/external_encoder.ml | 5 +---- src/core/encoder/encoders/fdkaac_encoder.ml | 2 +- .../encoder/encoders/ffmpeg_encoder_common.ml | 8 +++++++- src/core/encoder/encoders/flac_encoder.ml | 2 +- src/core/encoder/encoders/gstreamer_encoder.ml | 2 +- src/core/encoder/encoders/lame_encoder.ml | 2 +- src/core/encoder/encoders/ogg_encoder.ml | 18 ++++++------------ src/core/encoder/encoders/shine_encoder.ml | 2 +- src/core/encoder/encoders/wav_encoder.ml | 2 +- src/core/ogg_formats/ogg_muxer.ml | 1 + src/core/outputs/harbor_output.ml | 2 +- src/core/outputs/hls_output.ml | 2 +- 15 files changed, 27 insertions(+), 34 deletions(-) diff --git a/src/core/encoder/encoder.ml b/src/core/encoder/encoder.ml index 1396536b62..b5ed07c869 100644 --- a/src/core/encoder/encoder.ml +++ b/src/core/encoder/encoder.ml @@ -314,10 +314,7 @@ let dummy_hls encode = type encoder = { insert_metadata : Export_metadata.metadata -> unit; - (* Encoder are all called from the main - thread so there's no need to protect this - value with a mutex so far.. *) - mutable header : Strings.t; + header : unit -> Strings.t; hls : hls; encode : Frame.t -> int -> int -> Strings.t; stop : unit -> Strings.t; @@ -348,6 +345,7 @@ let get_factory fmt = (* Protect all functions with a mutex. *) let m = Mutex.create () in let insert_metadata = Tutils.mutexify m insert_metadata in + let header = Tutils.mutexify m header in let { init; init_encode; diff --git a/src/core/encoder/encoder.mli b/src/core/encoder/encoder.mli index d8ab0d0a6b..06cc789c51 100644 --- a/src/core/encoder/encoder.mli +++ b/src/core/encoder/encoder.mli @@ -110,10 +110,7 @@ val dummy_hls : (Generator.t -> int -> int -> Strings.t) -> hls type encoder = { insert_metadata : Export_metadata.metadata -> unit; - (* Encoder are all called from the main - * thread so there's no need to protect this - * value with a mutex so far.. *) - mutable header : Strings.t; + header : unit -> Strings.t; hls : hls; encode : Frame.t -> int -> int -> Strings.t; stop : unit -> Strings.t; diff --git a/src/core/encoder/encoders/avi_encoder.ml b/src/core/encoder/encoders/avi_encoder.ml index 1338592b18..cad3741704 100644 --- a/src/core/encoder/encoders/avi_encoder.ml +++ b/src/core/encoder/encoders/avi_encoder.ml @@ -116,7 +116,7 @@ let encoder ~pos:_ avi = Encoder.insert_metadata = (fun _ -> ()); hls = Encoder.dummy_hls encode; encode; - header = Strings.of_string header; + header = (fun () -> Strings.of_string header); stop = (fun () -> Strings.empty); } diff --git a/src/core/encoder/encoders/external_encoder.ml b/src/core/encoder/encoders/external_encoder.ml index 90a134ec84..1f4d81f989 100644 --- a/src/core/encoder/encoders/external_encoder.ml +++ b/src/core/encoder/encoders/external_encoder.ml @@ -164,10 +164,7 @@ let encoder ~pos:_ id ext = in { Encoder.insert_metadata; - (* External encoders do not support - * headers for now. They will probably - * never do.. *) - header = Strings.empty; + header = (fun () -> Strings.empty); hls = Encoder.dummy_hls encode; encode; stop; diff --git a/src/core/encoder/encoders/fdkaac_encoder.ml b/src/core/encoder/encoders/fdkaac_encoder.ml index 1ab8fe97a8..5673d72941 100644 --- a/src/core/encoder/encoders/fdkaac_encoder.ml +++ b/src/core/encoder/encoders/fdkaac_encoder.ml @@ -110,7 +110,7 @@ let encoder ~pos aac = in { Encoder.insert_metadata = (fun _ -> ()); - header = Strings.empty; + header = (fun () -> Strings.empty); hls = Encoder.dummy_hls encode; encode; stop; diff --git a/src/core/encoder/encoders/ffmpeg_encoder_common.ml b/src/core/encoder/encoders/ffmpeg_encoder_common.ml index d9e2ce2eef..3f9d8b565c 100644 --- a/src/core/encoder/encoders/ffmpeg_encoder_common.ml +++ b/src/core/encoder/encoders/ffmpeg_encoder_common.ml @@ -335,4 +335,10 @@ let encoder ~pos ~mk_streams ffmpeg meta = video_size; } in - { Encoder.insert_metadata; header = Strings.empty; hls; encode; stop } + { + Encoder.insert_metadata; + header = (fun () -> Strings.empty); + hls; + encode; + stop; + } diff --git a/src/core/encoder/encoders/flac_encoder.ml b/src/core/encoder/encoders/flac_encoder.ml index abf9f61ad0..feca85cfff 100644 --- a/src/core/encoder/encoders/flac_encoder.ml +++ b/src/core/encoder/encoders/flac_encoder.ml @@ -65,7 +65,7 @@ let encoder ~pos:_ flac meta = Encoder.insert_metadata = (fun _ -> ()); (* Flac encoder do not support header * for now. It will probably never do.. *) - header = Strings.empty; + header = (fun () -> Strings.empty); hls = Encoder.dummy_hls encode; encode; stop; diff --git a/src/core/encoder/encoders/gstreamer_encoder.ml b/src/core/encoder/encoders/gstreamer_encoder.ml index acc01d3cc9..408c5bb1f3 100644 --- a/src/core/encoder/encoders/gstreamer_encoder.ml +++ b/src/core/encoder/encoders/gstreamer_encoder.ml @@ -191,7 +191,7 @@ let encoder ext = in { Encoder.insert_metadata; - header = Strings.empty; + header = (fun () -> Strings.empty); hls = Encoder.dummy_hls encode; encode; stop; diff --git a/src/core/encoder/encoders/lame_encoder.ml b/src/core/encoder/encoders/lame_encoder.ml index 2bc4a65295..71875a30bf 100644 --- a/src/core/encoder/encoders/lame_encoder.ml +++ b/src/core/encoder/encoders/lame_encoder.ml @@ -119,7 +119,7 @@ let () = insert_metadata = (fun _ -> ()); hls = Encoder.dummy_hls encode; encode; - header = Strings.empty; + header = (fun () -> Strings.empty); stop; } in diff --git a/src/core/encoder/encoders/ogg_encoder.ml b/src/core/encoder/encoders/ogg_encoder.ml index 472e188ab9..ba80d1a766 100644 --- a/src/core/encoder/encoders/ogg_encoder.ml +++ b/src/core/encoder/encoders/ogg_encoder.ml @@ -105,7 +105,7 @@ let encoder ~pos { Ogg_format.audio; video } = Encoder.insert_metadata; hls = Encoder.dummy_hls (fun _ _ -> assert false); encode; - header = Strings.empty; + header = (fun () -> Ogg_muxer.get_header ogg_enc); stop; } and streams_start () = @@ -115,14 +115,11 @@ let encoder ~pos { Ogg_format.audio; video } = | None -> track.id <- Some (track.reset ogg_enc meta) in List.iter f tracks; - Ogg_muxer.streams_start ogg_enc; - enc.Encoder.header <- Ogg_muxer.get_header ogg_enc + Ogg_muxer.streams_start ogg_enc and encode frame start len = (* We do a lazy start, to * avoid empty streams at beginning.. *) - if Ogg_muxer.state ogg_enc <> Ogg_muxer.Streaming then ( - streams_start (); - enc.Encoder.header <- Ogg_muxer.get_header ogg_enc); + if Ogg_muxer.state ogg_enc <> Ogg_muxer.Streaming then streams_start (); let f track = track.encode ogg_enc (Option.get track.id) frame start len in @@ -131,18 +128,15 @@ let encoder ~pos { Ogg_format.audio; video } = and ogg_stop () = let f track = track.id <- None in List.iter f tracks; - if Ogg_muxer.state ogg_enc = Ogg_muxer.Streaming then ( - Ogg_muxer.end_of_stream ogg_enc; - enc.Encoder.header <- Strings.empty) + if Ogg_muxer.state ogg_enc = Ogg_muxer.Streaming then + Ogg_muxer.end_of_stream ogg_enc and stop () = ogg_stop (); - enc.Encoder.header <- Strings.empty; Ogg_muxer.get_data ogg_enc and insert_metadata m = ogg_stop (); let f track = track.id <- Some (track.reset ogg_enc m) in - List.iter f tracks; - enc.Encoder.header <- Ogg_muxer.get_header ogg_enc + List.iter f tracks in { enc with hls = Encoder.dummy_hls encode } diff --git a/src/core/encoder/encoders/shine_encoder.ml b/src/core/encoder/encoders/shine_encoder.ml index 70c3631049..cf93d96050 100644 --- a/src/core/encoder/encoders/shine_encoder.ml +++ b/src/core/encoder/encoders/shine_encoder.ml @@ -66,7 +66,7 @@ let encoder ~pos:_ shine = let stop () = Strings.of_string (Shine.flush enc) in { Encoder.insert_metadata = (fun _ -> ()); - header = Strings.empty; + header = (fun () -> Strings.empty); hls = Encoder.dummy_hls encode; encode; stop; diff --git a/src/core/encoder/encoders/wav_encoder.ml b/src/core/encoder/encoders/wav_encoder.ml index 8597c12c2e..0be4dc5700 100644 --- a/src/core/encoder/encoders/wav_encoder.ml +++ b/src/core/encoder/encoders/wav_encoder.ml @@ -73,7 +73,7 @@ let encoder ~pos wav = Encoder.insert_metadata = (fun _ -> ()); hls = Encoder.dummy_hls encode; encode; - header = Strings.of_string header; + header = (fun () -> Strings.of_string header); stop = (fun () -> Strings.empty); } diff --git a/src/core/ogg_formats/ogg_muxer.ml b/src/core/ogg_formats/ogg_muxer.ml index eef842796c..b28eb4d596 100644 --- a/src/core/ogg_formats/ogg_muxer.ml +++ b/src/core/ogg_formats/ogg_muxer.ml @@ -403,6 +403,7 @@ let eos encoder = if Hashtbl.length encoder.tracks <> 0 then raise Invalid_usage; log#info "%s: Every ogg logical tracks have ended: setting end of stream." encoder.id; + Printf.printf "Flushing header\n%!"; ignore (Strings.Mutable.flush encoder.header); encoder.position <- 0.; encoder.state <- Eos diff --git a/src/core/outputs/harbor_output.ml b/src/core/outputs/harbor_output.ml index 9a76ecd875..09a41866db 100644 --- a/src/core/outputs/harbor_output.ml +++ b/src/core/outputs/harbor_output.ml @@ -456,7 +456,7 @@ class output p = data.format icyheader extra_headers in let buffer = - Strings.Mutable.of_strings (Option.get encoder).Encoder.header + Strings.Mutable.of_strings ((Option.get encoder).Encoder.header ()) in let close () = try Harbor.close s with _ -> () in let rec client = diff --git a/src/core/outputs/hls_output.ml b/src/core/outputs/hls_output.ml index 4db4f47e37..cc5511da7d 100644 --- a/src/core/outputs/hls_output.ml +++ b/src/core/outputs/hls_output.ml @@ -591,7 +591,7 @@ class hls_output p = Lang.raise_as_runtime ~bt ~kind:"file" exn) in let out_channel = self#open_out filename in - Strings.iter (output_substring out_channel) s.encoder.Encoder.header; + Strings.iter (output_substring out_channel) (s.encoder.Encoder.header ()); let discontinuous = state = `Restarted in let segment = {