Message ID | 20210721172916.40739-1-jeebjp@gmail.com |
---|---|
State | Accepted |
Commit | 4c694093be68d401c60819e5171817c62afef8b2 |
Headers | show |
Series | [FFmpeg-devel] ffmpeg: fix order between field order autodetection and override | 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 |
On Wed, Jul 21, 2021 at 8:29 PM Jan Ekström <jeebjp@gmail.com> wrote: > > Somehow I missed this in fbb44bc51a647862eb05ae3f9d7d49a0be9bed57 , > even though the lines were within the context. Probably the code > originally being after the this logic had something to do with it, > but previously it only touched the avformat context's codecpar, > which did not affect the encoder codec context whatsoever. > > Fixes #9320 > Fixes #9339 > --- > fftools/ffmpeg.c | 14 ++++++++------ > 1 file changed, 8 insertions(+), 6 deletions(-) > > diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c > index 40e8010096..b0ce7c7c32 100644 > --- a/fftools/ffmpeg.c > +++ b/fftools/ffmpeg.c > @@ -3483,12 +3483,7 @@ static int init_output_stream_encode(OutputStream *ost, AVFrame *frame) > enc_ctx->bits_per_raw_sample = frame_bits_per_raw_sample; > } > > - if (ost->top_field_first == 0) { > - enc_ctx->field_order = AV_FIELD_BB; > - } else if (ost->top_field_first == 1) { > - enc_ctx->field_order = AV_FIELD_TT; > - } > - > + // Field order: autodetection > if (frame) { > if (enc_ctx->flags & (AV_CODEC_FLAG_INTERLACED_DCT | AV_CODEC_FLAG_INTERLACED_ME) && > ost->top_field_first >= 0) > @@ -3503,6 +3498,13 @@ static int init_output_stream_encode(OutputStream *ost, AVFrame *frame) > enc_ctx->field_order = AV_FIELD_PROGRESSIVE; > } > > + // Field order: override > + if (ost->top_field_first == 0) { > + enc_ctx->field_order = AV_FIELD_BB; > + } else if (ost->top_field_first == 1) { > + enc_ctx->field_order = AV_FIELD_TT; > + } > + > if (ost->forced_keyframes) { > if (!strncmp(ost->forced_keyframes, "expr:", 5)) { > ret = av_expr_parse(&ost->forced_keyframes_pexpr, ost->forced_keyframes+5, > -- > 2.31.1 > Ping.
On Wed, Jul 21, 2021 at 7:29 PM Jan Ekström <jeebjp@gmail.com> wrote: > Somehow I missed this in fbb44bc51a647862eb05ae3f9d7d49a0be9bed57 , > even though the lines were within the context. Probably the code > originally being after the this logic had something to do with it, > but previously it only touched the avformat context's codecpar, > which did not affect the encoder codec context whatsoever. > > Fixes #9320 > Fixes #9339 > --- > fftools/ffmpeg.c | 14 ++++++++------ > 1 file changed, 8 insertions(+), 6 deletions(-) > > diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c > index 40e8010096..b0ce7c7c32 100644 > --- a/fftools/ffmpeg.c > +++ b/fftools/ffmpeg.c > @@ -3483,12 +3483,7 @@ static int init_output_stream_encode(OutputStream > *ost, AVFrame *frame) > enc_ctx->bits_per_raw_sample = frame_bits_per_raw_sample; > } > > - if (ost->top_field_first == 0) { > - enc_ctx->field_order = AV_FIELD_BB; > - } else if (ost->top_field_first == 1) { > - enc_ctx->field_order = AV_FIELD_TT; > - } > - > + // Field order: autodetection > if (frame) { > if (enc_ctx->flags & (AV_CODEC_FLAG_INTERLACED_DCT | > AV_CODEC_FLAG_INTERLACED_ME) && > ost->top_field_first >= 0) > @@ -3503,6 +3498,13 @@ static int init_output_stream_encode(OutputStream > *ost, AVFrame *frame) > enc_ctx->field_order = AV_FIELD_PROGRESSIVE; > } > > + // Field order: override > + if (ost->top_field_first == 0) { > + enc_ctx->field_order = AV_FIELD_BB; > + } else if (ost->top_field_first == 1) { > + enc_ctx->field_order = AV_FIELD_TT; > + } > + > if (ost->forced_keyframes) { > if (!strncmp(ost->forced_keyframes, "expr:", 5)) { > ret = av_expr_parse(&ost->forced_keyframes_pexpr, > ost->forced_keyframes+5, > -- > 2.31.1 > > LGTM > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
On Sun, Jul 25, 2021 at 10:38 PM Paul B Mahol <onemda@gmail.com> wrote: > > On Wed, Jul 21, 2021 at 7:29 PM Jan Ekström <jeebjp@gmail.com> wrote: > > > Somehow I missed this in fbb44bc51a647862eb05ae3f9d7d49a0be9bed57 , > > even though the lines were within the context. Probably the code > > originally being after the this logic had something to do with it, > > but previously it only touched the avformat context's codecpar, > > which did not affect the encoder codec context whatsoever. > > > > Fixes #9320 > > Fixes #9339 > > --- > > fftools/ffmpeg.c | 14 ++++++++------ > > 1 file changed, 8 insertions(+), 6 deletions(-) > > > > diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c > > index 40e8010096..b0ce7c7c32 100644 > > --- a/fftools/ffmpeg.c > > +++ b/fftools/ffmpeg.c > > @@ -3483,12 +3483,7 @@ static int init_output_stream_encode(OutputStream > > *ost, AVFrame *frame) > > enc_ctx->bits_per_raw_sample = frame_bits_per_raw_sample; > > } > > > > - if (ost->top_field_first == 0) { > > - enc_ctx->field_order = AV_FIELD_BB; > > - } else if (ost->top_field_first == 1) { > > - enc_ctx->field_order = AV_FIELD_TT; > > - } > > - > > + // Field order: autodetection > > if (frame) { > > if (enc_ctx->flags & (AV_CODEC_FLAG_INTERLACED_DCT | > > AV_CODEC_FLAG_INTERLACED_ME) && > > ost->top_field_first >= 0) > > @@ -3503,6 +3498,13 @@ static int init_output_stream_encode(OutputStream > > *ost, AVFrame *frame) > > enc_ctx->field_order = AV_FIELD_PROGRESSIVE; > > } > > > > + // Field order: override > > + if (ost->top_field_first == 0) { > > + enc_ctx->field_order = AV_FIELD_BB; > > + } else if (ost->top_field_first == 1) { > > + enc_ctx->field_order = AV_FIELD_TT; > > + } > > + > > if (ost->forced_keyframes) { > > if (!strncmp(ost->forced_keyframes, "expr:", 5)) { > > ret = av_expr_parse(&ost->forced_keyframes_pexpr, > > ost->forced_keyframes+5, > > -- > > 2.31.1 > > > > > LGTM > Thanks, applied with the recommendations you made for the commit message, as well as fixing a case of "the this logic". Jan
diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c index 40e8010096..b0ce7c7c32 100644 --- a/fftools/ffmpeg.c +++ b/fftools/ffmpeg.c @@ -3483,12 +3483,7 @@ static int init_output_stream_encode(OutputStream *ost, AVFrame *frame) enc_ctx->bits_per_raw_sample = frame_bits_per_raw_sample; } - if (ost->top_field_first == 0) { - enc_ctx->field_order = AV_FIELD_BB; - } else if (ost->top_field_first == 1) { - enc_ctx->field_order = AV_FIELD_TT; - } - + // Field order: autodetection if (frame) { if (enc_ctx->flags & (AV_CODEC_FLAG_INTERLACED_DCT | AV_CODEC_FLAG_INTERLACED_ME) && ost->top_field_first >= 0) @@ -3503,6 +3498,13 @@ static int init_output_stream_encode(OutputStream *ost, AVFrame *frame) enc_ctx->field_order = AV_FIELD_PROGRESSIVE; } + // Field order: override + if (ost->top_field_first == 0) { + enc_ctx->field_order = AV_FIELD_BB; + } else if (ost->top_field_first == 1) { + enc_ctx->field_order = AV_FIELD_TT; + } + if (ost->forced_keyframes) { if (!strncmp(ost->forced_keyframes, "expr:", 5)) { ret = av_expr_parse(&ost->forced_keyframes_pexpr, ost->forced_keyframes+5,