From 904538721cf2b69bb8d80059d13ae20e4f256fce Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Sat, 19 Oct 2024 20:20:00 +0200 Subject: Tidy up and fix obtaining video length when it's not in the header. I think this has been broken for a while as we would come out of the loop even if we still wanted to find the video length. --- src/lib/ffmpeg_examiner.cc | 26 ++++++++++++++------------ src/lib/ffmpeg_examiner.h | 2 +- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/src/lib/ffmpeg_examiner.cc b/src/lib/ffmpeg_examiner.cc index 51ade8e89..6d3b61473 100644 --- a/src/lib/ffmpeg_examiner.cc +++ b/src/lib/ffmpeg_examiner.cc @@ -144,24 +144,25 @@ FFmpegExaminer::FFmpegExaminer (shared_ptr c, shared_ptrstream_index]; + auto carry_on = false; + if (_video_stream && packet->stream_index == _video_stream.get()) { - video_packet (context, temporal_reference, packet); + if (video_packet(context, temporal_reference, packet)) { + carry_on = true; + } } - bool got_all_audio = true; - for (size_t i = 0; i < _audio_streams.size(); ++i) { if (_audio_streams[i]->uses_index(_format_context, packet->stream_index)) { - audio_packet (context, _audio_streams[i], packet); - } - if (!_audio_streams[i]->first_audio) { - got_all_audio = false; + if (audio_packet(context, _audio_streams[i], packet)) { + carry_on = true; + } } } av_packet_free (&packet); - if (_first_video && got_all_audio && temporal_reference.size() >= (PULLDOWN_CHECK_FRAMES * 2)) { + if (!carry_on) { /* All done */ break; } @@ -261,26 +262,27 @@ FFmpegExaminer::video_packet (AVCodecContext* context, string& temporal_referenc } -void +bool FFmpegExaminer::audio_packet (AVCodecContext* context, shared_ptr stream, AVPacket* packet) { if (stream->first_audio) { - return; + return false; } int r = avcodec_send_packet (context, packet); if (r < 0) { LOG_WARNING("avcodec_send_packet returned %1 for an audio packet", r); - return; + return false; } auto frame = audio_frame (stream); if (avcodec_receive_frame (context, frame) < 0) { - return; + return false; } stream->first_audio = frame_time (frame, stream->stream(_format_context)); + return true; } diff --git a/src/lib/ffmpeg_examiner.h b/src/lib/ffmpeg_examiner.h index 45313ec18..b2dcbb27d 100644 --- a/src/lib/ffmpeg_examiner.h +++ b/src/lib/ffmpeg_examiner.h @@ -89,7 +89,7 @@ public: private: bool video_packet (AVCodecContext* context, std::string& temporal_reference, AVPacket* packet); - void audio_packet (AVCodecContext* context, std::shared_ptr, AVPacket* packet); + bool audio_packet (AVCodecContext* context, std::shared_ptr, AVPacket* packet); std::string stream_name (AVStream* s) const; std::string subtitle_stream_name (AVStream* s) const; -- cgit v1.2.3 From 027dc03440ff0714b541b810264efa8722c87f39 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Sat, 19 Oct 2024 20:32:56 +0200 Subject: Cleanup: initialise a few members in-place. --- src/lib/ffmpeg_examiner.cc | 3 --- src/lib/ffmpeg_examiner.h | 6 +++--- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/lib/ffmpeg_examiner.cc b/src/lib/ffmpeg_examiner.cc index 6d3b61473..b61089269 100644 --- a/src/lib/ffmpeg_examiner.cc +++ b/src/lib/ffmpeg_examiner.cc @@ -62,9 +62,6 @@ static const int PULLDOWN_CHECK_FRAMES = 16; /** @param job job that the examiner is operating in, or 0 */ FFmpegExaminer::FFmpegExaminer (shared_ptr c, shared_ptr job) : FFmpeg (c) - , _video_length (0) - , _need_video_length (false) - , _pulldown (false) { /* Find audio and subtitle streams */ diff --git a/src/lib/ffmpeg_examiner.h b/src/lib/ffmpeg_examiner.h index b2dcbb27d..f6fe8c423 100644 --- a/src/lib/ffmpeg_examiner.h +++ b/src/lib/ffmpeg_examiner.h @@ -101,11 +101,11 @@ private: /** Video length, either obtained from the header or derived by running * through the whole file. */ - Frame _video_length; - bool _need_video_length; + Frame _video_length = 0; + bool _need_video_length = false; boost::optional _rotation; - bool _pulldown; + bool _pulldown = false; struct SubtitleStart { -- cgit v1.2.3 From 24cfca51dffed98f0b115ca36f9a8478753e2432 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Sat, 19 Oct 2024 21:29:29 +0200 Subject: Fix video length when we scan for it. Length is 1 frame more than the start time of the last frame. --- src/lib/ffmpeg_examiner.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/ffmpeg_examiner.cc b/src/lib/ffmpeg_examiner.cc index b61089269..31f1a3d64 100644 --- a/src/lib/ffmpeg_examiner.cc +++ b/src/lib/ffmpeg_examiner.cc @@ -245,10 +245,10 @@ FFmpegExaminer::video_packet (AVCodecContext* context, string& temporal_referenc _first_video = frame_time (_video_frame, _format_context->streams[_video_stream.get()]); } if (_need_video_length) { - _video_length = frame_time ( + _video_length = frame_time( _video_frame, _format_context->streams[_video_stream.get()] - ).get_value_or (ContentTime ()).frames_round (video_frame_rate().get ()); + ).get_value_or({}).frames_round(video_frame_rate().get()) + 1; } if (temporal_reference.size() < (PULLDOWN_CHECK_FRAMES * 2)) { temporal_reference += (_video_frame->top_field_first ? "T" : "B"); -- cgit v1.2.3 From 738ced15ead0f029cf2becf9b77d7adf37e733e3 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Sat, 19 Oct 2024 21:29:54 +0200 Subject: Obtain audio length by scanning through the file if required (#2880). --- run/tests | 2 +- src/lib/audio_stream.h | 5 ++++ src/lib/ffmpeg_examiner.cc | 57 ++++++++++++++++++++++++++++++++-------------- src/lib/ffmpeg_examiner.h | 1 + test/ffmpeg_audio_test.cc | 8 +++++++ 5 files changed, 55 insertions(+), 18 deletions(-) diff --git a/run/tests b/run/tests index 5f356add3..e1542c114 100755 --- a/run/tests +++ b/run/tests @@ -3,7 +3,7 @@ # e.g. --run_tests=foo set -e -PRIVATE_GIT="881c48805e352dfe150993814757ca974282be18" +PRIVATE_GIT="65c6d0cf0ec188e4d53fc8c9d61958c7921219d6" type="" check=0 diff --git a/src/lib/audio_stream.h b/src/lib/audio_stream.h index cf874242f..b125eb8c0 100644 --- a/src/lib/audio_stream.h +++ b/src/lib/audio_stream.h @@ -54,6 +54,11 @@ public: return _length; } + void set_length(Frame length) { + boost::mutex::scoped_lock lm (_mutex); + _length = length; + } + int channels () const; boost::optional bit_depth() const; diff --git a/src/lib/ffmpeg_examiner.cc b/src/lib/ffmpeg_examiner.cc index 31f1a3d64..810a6eba2 100644 --- a/src/lib/ffmpeg_examiner.cc +++ b/src/lib/ffmpeg_examiner.cc @@ -78,7 +78,10 @@ FFmpegExaminer::FFmpegExaminer (shared_ptr c, shared_ptrcodecpar->channel_layout = av_get_default_channel_layout (s->codecpar->channels); } - DCPOMATIC_ASSERT (_format_context->duration != AV_NOPTS_VALUE); + if (_format_context->duration == AV_NOPTS_VALUE) { + _need_audio_length = true; + } + DCPOMATIC_ASSERT (codec->name); _audio_streams.push_back ( @@ -87,7 +90,7 @@ FFmpegExaminer::FFmpegExaminer (shared_ptr c, shared_ptrname, s->id, s->codecpar->sample_rate, - llrint ((double(_format_context->duration) / AV_TIME_BASE) * s->codecpar->sample_rate), + _need_audio_length ? 0 : rint ((double(_format_context->duration) / AV_TIME_BASE) * s->codecpar->sample_rate), s->codecpar->channels, s->codecpar->bits_per_raw_sample ? s->codecpar->bits_per_raw_sample : s->codecpar->bits_per_coded_sample ) @@ -114,6 +117,7 @@ FFmpegExaminer::FFmpegExaminer (shared_ptr c, shared_ptr c, shared_ptr carry_on_audio(_audio_streams.size()); while (true) { auto packet = av_packet_alloc (); DCPOMATIC_ASSERT (packet); @@ -141,27 +147,37 @@ FFmpegExaminer::FFmpegExaminer (shared_ptr c, shared_ptrstream_index]; - auto carry_on = false; - - if (_video_stream && packet->stream_index == _video_stream.get()) { - if (video_packet(context, temporal_reference, packet)) { - carry_on = true; + boost::optional audio_stream_index; + for (size_t i = 0; i < _audio_streams.size(); ++i) { + if (_audio_streams[i]->uses_index(_format_context, packet->stream_index)) { + audio_stream_index = i; } } - for (size_t i = 0; i < _audio_streams.size(); ++i) { - if (_audio_streams[i]->uses_index(_format_context, packet->stream_index)) { - if (audio_packet(context, _audio_streams[i], packet)) { - carry_on = true; - } + bool const video = _video_stream && packet->stream_index == *_video_stream; + + if (!video && !audio_stream_index) { + av_packet_free(&packet); + continue; + } + + if (video) { + carry_on_video = video_packet(context, temporal_reference, packet); + } + + if (audio_stream_index) { + if (audio_packet(context, _audio_streams[*audio_stream_index], packet)) { + carry_on_audio[*audio_stream_index] = true; } } av_packet_free (&packet); - if (!carry_on) { - /* All done */ - break; + if (!carry_on_video) { + if (std::find(carry_on_audio.begin(), carry_on_audio.end(), true) == carry_on_audio.end()) { + /* All done */ + break; + } } } @@ -262,7 +278,7 @@ FFmpegExaminer::video_packet (AVCodecContext* context, string& temporal_referenc bool FFmpegExaminer::audio_packet (AVCodecContext* context, shared_ptr stream, AVPacket* packet) { - if (stream->first_audio) { + if (stream->first_audio && !_need_audio_length) { return false; } @@ -278,7 +294,14 @@ FFmpegExaminer::audio_packet (AVCodecContext* context, shared_ptrfirst_audio = frame_time (frame, stream->stream(_format_context)); + if (!stream->first_audio) { + stream->first_audio = frame_time(frame, stream->stream(_format_context)); + } + + if (_need_audio_length) { + stream->set_length(frame_time(frame, stream->stream(_format_context)).get_value_or({}).frames_round(stream->frame_rate()) + frame->nb_samples); + } + return true; } diff --git a/src/lib/ffmpeg_examiner.h b/src/lib/ffmpeg_examiner.h index f6fe8c423..57c97c542 100644 --- a/src/lib/ffmpeg_examiner.h +++ b/src/lib/ffmpeg_examiner.h @@ -103,6 +103,7 @@ private: */ Frame _video_length = 0; bool _need_video_length = false; + bool _need_audio_length = false; boost::optional _rotation; bool _pulldown = false; diff --git a/test/ffmpeg_audio_test.cc b/test/ffmpeg_audio_test.cc index b2a83aad8..9ab7666c7 100644 --- a/test/ffmpeg_audio_test.cc +++ b/test/ffmpeg_audio_test.cc @@ -140,3 +140,11 @@ BOOST_AUTO_TEST_CASE (ffmpeg_audio_test4) BOOST_CHECK_NO_THROW (while (!player->pass()) {}); } + + +BOOST_AUTO_TEST_CASE(no_audio_length_in_header) +{ + auto content = content_factory(TestPaths::private_data() / "10-seconds.thd"); + auto film = new_test_film2("no_audio_length_in_header", content); + BOOST_CHECK(content[0]->full_length(film) == dcpomatic::DCPTime::from_seconds(10)); +} -- cgit v1.2.3 From 8edadeea58faf5c075296df1f605407797e9a25a Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Mon, 28 Oct 2024 01:36:55 +0100 Subject: Needing length is the same for audio and video, so merge them. --- src/lib/ffmpeg.cc | 2 ++ src/lib/ffmpeg_examiner.cc | 22 ++++++++-------------- src/lib/ffmpeg_examiner.h | 3 +-- 3 files changed, 11 insertions(+), 16 deletions(-) diff --git a/src/lib/ffmpeg.cc b/src/lib/ffmpeg.cc index d9df232df..550e045b3 100644 --- a/src/lib/ffmpeg.cc +++ b/src/lib/ffmpeg.cc @@ -326,6 +326,8 @@ FFmpeg::pts_offset (vector> audio_streams, optiona } } + DCPOMATIC_ASSERT(po != ContentTime::min()); + /* If the offset is positive we would be pushing things from a -ve PTS to be played. I don't think we ever want to do that, as it seems things at -ve PTS are not meant to be seen (use for alignment bars etc.); see mantis #418. diff --git a/src/lib/ffmpeg_examiner.cc b/src/lib/ffmpeg_examiner.cc index 810a6eba2..3de1be888 100644 --- a/src/lib/ffmpeg_examiner.cc +++ b/src/lib/ffmpeg_examiner.cc @@ -63,6 +63,7 @@ static const int PULLDOWN_CHECK_FRAMES = 16; FFmpegExaminer::FFmpegExaminer (shared_ptr c, shared_ptr job) : FFmpeg (c) { + _need_length = _format_context->duration == AV_NOPTS_VALUE; /* Find audio and subtitle streams */ for (uint32_t i = 0; i < _format_context->nb_streams; ++i) { @@ -78,10 +79,6 @@ FFmpegExaminer::FFmpegExaminer (shared_ptr c, shared_ptrcodecpar->channel_layout = av_get_default_channel_layout (s->codecpar->channels); } - if (_format_context->duration == AV_NOPTS_VALUE) { - _need_audio_length = true; - } - DCPOMATIC_ASSERT (codec->name); _audio_streams.push_back ( @@ -90,7 +87,7 @@ FFmpegExaminer::FFmpegExaminer (shared_ptr c, shared_ptrname, s->id, s->codecpar->sample_rate, - _need_audio_length ? 0 : rint ((double(_format_context->duration) / AV_TIME_BASE) * s->codecpar->sample_rate), + _need_length ? 0 : rint ((double(_format_context->duration) / AV_TIME_BASE) * s->codecpar->sample_rate), s->codecpar->channels, s->codecpar->bits_per_raw_sample ? s->codecpar->bits_per_raw_sample : s->codecpar->bits_per_coded_sample ) @@ -103,13 +100,10 @@ FFmpegExaminer::FFmpegExaminer (shared_ptr c, shared_ptrduration == AV_NOPTS_VALUE; - if (!_need_video_length) { - _video_length = llrint ((double (_format_context->duration) / AV_TIME_BASE) * video_frame_rate().get()); - } + _video_length = _need_length ? 0 : llrint((double (_format_context->duration) / AV_TIME_BASE) * video_frame_rate().get()); } - if (job && _need_video_length) { + if (job && _need_length) { job->sub (_("Finding length")); } @@ -233,7 +227,7 @@ FFmpegExaminer::video_packet (AVCodecContext* context, string& temporal_referenc { DCPOMATIC_ASSERT (_video_stream); - if (_first_video && !_need_video_length && temporal_reference.size() >= (PULLDOWN_CHECK_FRAMES * 2)) { + if (_first_video && !_need_length && temporal_reference.size() >= (PULLDOWN_CHECK_FRAMES * 2)) { return false; } @@ -260,7 +254,7 @@ FFmpegExaminer::video_packet (AVCodecContext* context, string& temporal_referenc if (!_first_video) { _first_video = frame_time (_video_frame, _format_context->streams[_video_stream.get()]); } - if (_need_video_length) { + if (_need_length) { _video_length = frame_time( _video_frame, _format_context->streams[_video_stream.get()] @@ -278,7 +272,7 @@ FFmpegExaminer::video_packet (AVCodecContext* context, string& temporal_referenc bool FFmpegExaminer::audio_packet (AVCodecContext* context, shared_ptr stream, AVPacket* packet) { - if (stream->first_audio && !_need_audio_length) { + if (stream->first_audio && !_need_length) { return false; } @@ -298,7 +292,7 @@ FFmpegExaminer::audio_packet (AVCodecContext* context, shared_ptrfirst_audio = frame_time(frame, stream->stream(_format_context)); } - if (_need_audio_length) { + if (_need_length) { stream->set_length(frame_time(frame, stream->stream(_format_context)).get_value_or({}).frames_round(stream->frame_rate()) + frame->nb_samples); } diff --git a/src/lib/ffmpeg_examiner.h b/src/lib/ffmpeg_examiner.h index 57c97c542..ad64f349d 100644 --- a/src/lib/ffmpeg_examiner.h +++ b/src/lib/ffmpeg_examiner.h @@ -102,8 +102,7 @@ private: * through the whole file. */ Frame _video_length = 0; - bool _need_video_length = false; - bool _need_audio_length = false; + bool _need_length = false; boost::optional _rotation; bool _pulldown = false; -- cgit v1.2.3