| Message ID | 20210124150556.1089358-1-linjie.justin.fu@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] lavf/utils: reset event_flags if extradata is not extracted correctly | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
| andriy/PPC64_make | success | Make finished |
| andriy/PPC64_make_fate | success | Make fate finished |
Quoting Linjie Fu (2021-01-24 16:05:56) > Regression since 87f0c8280. > > If the extradata of a stream could not be extracted correctly, > codec_info_nb_frames would remain zero, while st->event_flag would still > be marked as AVSTREAM_EVENT_FLAG_NEW_PACKETS. > > The two expressions could be different in this case, hence reset > event_flags and calculate the correct score. > > Fix #9029. The ticket mentions ffplay, but ffplay does not access event_flags.
On Mon, Jan 25, 2021 at 12:49 AM Anton Khirnov <anton@khirnov.net> wrote: > > Quoting Linjie Fu (2021-01-24 16:05:56) > > Regression since 87f0c8280. > > > > If the extradata of a stream could not be extracted correctly, > > codec_info_nb_frames would remain zero, while st->event_flag would still > > be marked as AVSTREAM_EVENT_FLAG_NEW_PACKETS. > > > > The two expressions could be different in this case, hence reset > > event_flags and calculate the correct score. > > > > Fix #9029. > > The ticket mentions ffplay, but ffplay does not access event_flags. > You are right, this helps ffmpeg cmdline to copy and dump: $ ffmpeg -i sample_cut.flv -c copy -y dump.mp4 Before 87f0c8280 and after this patch: audio streams could be dumped and playable, while video streams still not. 87f0c8280..master: Error reporting and quits. [mp4 @ 0x7fb276809a00] dimensions not set Could not write header for output file #0 (incorrect codec parameters ?): Invalid argument Error initializing output stream 0:1 -- I'd change the commit message to "#9029 related" for this, since the ffplay issue remains the same. - linjie
Quoting Linjie Fu (2021-01-25 04:49:21) > On Mon, Jan 25, 2021 at 12:49 AM Anton Khirnov <anton@khirnov.net> wrote: > > > > Quoting Linjie Fu (2021-01-24 16:05:56) > > > Regression since 87f0c8280. > > > > > > If the extradata of a stream could not be extracted correctly, > > > codec_info_nb_frames would remain zero, while st->event_flag would still > > > be marked as AVSTREAM_EVENT_FLAG_NEW_PACKETS. > > > > > > The two expressions could be different in this case, hence reset > > > event_flags and calculate the correct score. > > > > > > Fix #9029. > > > > The ticket mentions ffplay, but ffplay does not access event_flags. > > > > You are right, this helps ffmpeg cmdline to copy and dump: > $ ffmpeg -i sample_cut.flv -c copy -y dump.mp4 > > Before 87f0c8280 and after this patch: > > audio streams could be dumped and playable, while video streams still not. > > 87f0c8280..master: > Error reporting and quits. > > [mp4 @ 0x7fb276809a00] dimensions not set > Could not write header for output file #0 (incorrect codec parameters > ?): Invalid argument > Error initializing output stream 0:1 -- > > I'd change the commit message to "#9029 related" for this, since the > ffplay issue remains the same. I am still not convinced this is really a regression. You can still process the file by explicitly mapping only the audio stream. This file is damaged and the video stream is not readable. IMO it is better to report an error and let the user decide what to do with it than silently skip the video stream while pretending everything is fine.
On Mon, Jan 25, 2021 at 9:08 PM Anton Khirnov <anton@khirnov.net> wrote: > > Quoting Linjie Fu (2021-01-25 04:49:21) > > On Mon, Jan 25, 2021 at 12:49 AM Anton Khirnov <anton@khirnov.net> wrote: > > > > > > Quoting Linjie Fu (2021-01-24 16:05:56) > > > > Regression since 87f0c8280. > > > > > > > > If the extradata of a stream could not be extracted correctly, > > > > codec_info_nb_frames would remain zero, while st->event_flag would still > > > > be marked as AVSTREAM_EVENT_FLAG_NEW_PACKETS. > > > > > > > > The two expressions could be different in this case, hence reset > > > > event_flags and calculate the correct score. > > > > > > > > Fix #9029. > > > > > > The ticket mentions ffplay, but ffplay does not access event_flags. > > > > > > > You are right, this helps ffmpeg cmdline to copy and dump: > > $ ffmpeg -i sample_cut.flv -c copy -y dump.mp4 > > > > Before 87f0c8280 and after this patch: > > > > audio streams could be dumped and playable, while video streams still not. > > > > 87f0c8280..master: > > Error reporting and quits. > > > > [mp4 @ 0x7fb276809a00] dimensions not set > > Could not write header for output file #0 (incorrect codec parameters > > ?): Invalid argument > > Error initializing output stream 0:1 -- > > > > I'd change the commit message to "#9029 related" for this, since the > > ffplay issue remains the same. > > I am still not convinced this is really a regression. You can still > process the file by explicitly mapping only the audio stream. Okay, I'm fine with this if it won't trigger failures for to different paths. > This file is damaged and the video stream is not readable. IMO it is > better to report an error and let the user decide what to do with it > than silently skip the video stream while pretending everything is fine. VLC is able to decode/play this correctly, hence maybe need to investigate more into this. - linjie
Quoting Linjie Fu (2021-01-30 07:44:09) > On Mon, Jan 25, 2021 at 9:08 PM Anton Khirnov <anton@khirnov.net> wrote: > > > > Quoting Linjie Fu (2021-01-25 04:49:21) > > > On Mon, Jan 25, 2021 at 12:49 AM Anton Khirnov <anton@khirnov.net> wrote: > > > > > > > > Quoting Linjie Fu (2021-01-24 16:05:56) > > > > > Regression since 87f0c8280. > > > > > > > > > > If the extradata of a stream could not be extracted correctly, > > > > > codec_info_nb_frames would remain zero, while st->event_flag would still > > > > > be marked as AVSTREAM_EVENT_FLAG_NEW_PACKETS. > > > > > > > > > > The two expressions could be different in this case, hence reset > > > > > event_flags and calculate the correct score. > > > > > > > > > > Fix #9029. > > > > > > > > The ticket mentions ffplay, but ffplay does not access event_flags. > > > > > > > > > > You are right, this helps ffmpeg cmdline to copy and dump: > > > $ ffmpeg -i sample_cut.flv -c copy -y dump.mp4 > > > > > > Before 87f0c8280 and after this patch: > > > > > > audio streams could be dumped and playable, while video streams still not. > > > > > > 87f0c8280..master: > > > Error reporting and quits. > > > > > > [mp4 @ 0x7fb276809a00] dimensions not set > > > Could not write header for output file #0 (incorrect codec parameters > > > ?): Invalid argument > > > Error initializing output stream 0:1 -- > > > > > > I'd change the commit message to "#9029 related" for this, since the > > > ffplay issue remains the same. > > > > I am still not convinced this is really a regression. You can still > > process the file by explicitly mapping only the audio stream. > Okay, I'm fine with this if it won't trigger failures for to different paths. > > > This file is damaged and the video stream is not readable. IMO it is > > better to report an error and let the user decide what to do with it > > than silently skip the video stream while pretending everything is fine. > VLC is able to decode/play this correctly, hence maybe need to > investigate more into this. VLC can also play only the audio stream. ffmpeg handles it as I described above, which seems appropriate to me. I suppose ffplay could be changed to be more lenient about decoding errors, but then I see ffplay mainly as a testing/development tool rather than a serious player.
diff --git a/libavformat/utils.c b/libavformat/utils.c index 6f100294a1..de397a209e 100644 --- a/libavformat/utils.c +++ b/libavformat/utils.c @@ -3895,8 +3895,10 @@ FF_ENABLE_DEPRECATION_WARNINGS } if (!st->internal->avctx->extradata) { ret = extract_extradata(st, pkt); - if (ret < 0) + if (ret < 0) { + st->event_flags = 0; goto unref_then_goto_end; + } } /* If still no information, we try to open the codec and to
Regression since 87f0c8280. If the extradata of a stream could not be extracted correctly, codec_info_nb_frames would remain zero, while st->event_flag would still be marked as AVSTREAM_EVENT_FLAG_NEW_PACKETS. The two expressions could be different in this case, hence reset event_flags and calculate the correct score. Fix #9029. Signed-off-by: Linjie Fu <linjie.justin.fu@gmail.com> --- libavformat/utils.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)