diff mbox

[FFmpeg-devel,V2] ffmpeg: fix video_delay warning for HEVC/MPEG4 decoding

Message ID 1537956199-29213-1-git-send-email-mypopydev@gmail.com
State New
Headers show

Commit Message

Jun Zhao Sept. 26, 2018, 10:03 a.m. UTC 
For HEVC/MPEG4, we also need video_delay from the decoder, when decoding
some HEVC/MPEG4 clips, got numerous log like:
"video_delay is larger in decoder than demuxer", similar ticket: #3711

fix ticket: #6019

Signed-off-by: Lin Xie<lin.xie@intel.com>
Signed-off-by: Jun Zhao <jun.zhao@intel.com>
---
 fftools/ffmpeg.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

Comments

Michael Niedermayer Sept. 26, 2018, 8:09 p.m. UTC | #1 
On Wed, Sep 26, 2018 at 06:03:19PM +0800, Jun Zhao wrote:
> For HEVC/MPEG4, we also need video_delay from the decoder, when decoding
> some HEVC/MPEG4 clips, got numerous log like:
> "video_delay is larger in decoder than demuxer", similar ticket: #3711

for mpeg4 this looks incorrect. 

What your patch does is simply override the "demuxer side" but the patch
never explains why this would be the correct solution or why the demuxer
side gets the value wrong

Obviously if something (like the "demuxer side") gets a value wrong the
obvious solution is to fix that. For H264 determining the delay is rather
hard in some corner cases (as a result of the field that would signal the
delay being optional and not available in all streams) 

So please either fix the code that is responsible for this being set
wrong in mpeg4 or explain why we should hack around it instead of
fixing it.

thx

[...]
mypopy@gmail.com Sept. 27, 2018, 12:44 a.m. UTC | #2 
On Thu, Sep 27, 2018 at 4:09 AM Michael Niedermayer <michael@niedermayer.cc>
wrote:

> On Wed, Sep 26, 2018 at 06:03:19PM +0800, Jun Zhao wrote:
> > For HEVC/MPEG4, we also need video_delay from the decoder, when decoding
> > some HEVC/MPEG4 clips, got numerous log like:
> > "video_delay is larger in decoder than demuxer", similar ticket: #3711
>
> for mpeg4 this looks incorrect.
>
> What your patch does is simply override the "demuxer side" but the patch
> never explains why this would be the correct solution or why the demuxer
> side gets the value wrong
>
> Obviously if something (like the "demuxer side") gets a value wrong the
> obvious solution is to fix that. For H264 determining the delay is rather
> hard in some corner cases (as a result of the field that would signal the
> delay being optional and not available in all streams)
>
> So please either fix the code that is responsible for this being set
> wrong in mpeg4 or explain why we should hack around it instead of
> fixing it.
>
> thx
>
> So I need to dig in and root cause, I agree.
diff mbox

Patch

diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index 934dc71..7e939d0 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -2389,7 +2389,9 @@  static int decode_video(InputStream *ist, AVPacket *pkt, int *got_output, int64_
     // The following line may be required in some cases where there is no parser
     // or the parser does not has_b_frames correctly
     if (ist->st->codecpar->video_delay < ist->dec_ctx->has_b_frames) {
-        if (ist->dec_ctx->codec_id == AV_CODEC_ID_H264) {
+        if (ist->dec_ctx->codec_id == AV_CODEC_ID_H264 ||
+            ist->dec_ctx->codec_id == AV_CODEC_ID_HEVC ||
+            ist->dec_ctx->codec_id == AV_CODEC_ID_MPEG4) {
             ist->st->codecpar->video_delay = ist->dec_ctx->has_b_frames;
         } else
             av_log(ist->dec_ctx, AV_LOG_WARNING,