| Message ID | 20200925144318.6194-5-jamrial@gmail.com |
|---|---|
| State | Accepted |
| Commit | ea4b10249d1a9211fb050961d99aeb1725f4f783 |
| Headers | show |
| Series | [FFmpeg-devel,1/8] avcodec/cbs: add a flush callback to CodedBitstreamType | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
On 25/09/2020 15:43, James Almer wrote: > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavcodec/av1dec.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c > index 0bb04a3e44..f6b9fbbac3 100644 > --- a/libavcodec/av1dec.c > +++ b/libavcodec/av1dec.c > @@ -405,6 +405,9 @@ static av_cold int av1_decode_free(AVCodecContext *avctx) > static int set_context_with_sequence(AVCodecContext *avctx, > const AV1RawSequenceHeader *seq) > { > + int width = seq->max_frame_width_minus_1 + 1; > + int height = seq->max_frame_height_minus_1 + 1; > + > avctx->profile = seq->seq_profile; > avctx->level = seq->seq_level_idx[0]; > > @@ -423,6 +426,13 @@ static int set_context_with_sequence(AVCodecContext *avctx, > break; > } > > + if (avctx->width != width || avctx->height != height) { > + int ret = ff_set_dimensions(avctx, width, height); > + if (ret < 0) > + return ret; > + } > + avctx->sample_aspect_ratio = (AVRational) { 1, 1 }; > + > if (seq->timing_info.num_units_in_display_tick && > seq->timing_info.time_scale) { > av_reduce(&avctx->framerate.den, &avctx->framerate.num, > Can you explain more about why this might be wanted? There is no requirement that the stream actually contains any frames with the size stated in the sequence header. - Mark
On 9/29/2020 12:52 PM, Mark Thompson wrote: > On 25/09/2020 15:43, James Almer wrote: >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> libavcodec/av1dec.c | 10 ++++++++++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c >> index 0bb04a3e44..f6b9fbbac3 100644 >> --- a/libavcodec/av1dec.c >> +++ b/libavcodec/av1dec.c >> @@ -405,6 +405,9 @@ static av_cold int av1_decode_free(AVCodecContext >> *avctx) >> static int set_context_with_sequence(AVCodecContext *avctx, >> const AV1RawSequenceHeader *seq) >> { >> + int width = seq->max_frame_width_minus_1 + 1; >> + int height = seq->max_frame_height_minus_1 + 1; >> + >> avctx->profile = seq->seq_profile; >> avctx->level = seq->seq_level_idx[0]; >> @@ -423,6 +426,13 @@ static int >> set_context_with_sequence(AVCodecContext *avctx, >> break; >> } >> + if (avctx->width != width || avctx->height != height) { >> + int ret = ff_set_dimensions(avctx, width, height); >> + if (ret < 0) >> + return ret; >> + } >> + avctx->sample_aspect_ratio = (AVRational) { 1, 1 }; >> + >> if (seq->timing_info.num_units_in_display_tick && >> seq->timing_info.time_scale) { >> av_reduce(&avctx->framerate.den, &avctx->framerate.num, >> > > Can you explain more about why this might be wanted? There is no > requirement that the stream actually contains any frames with the size > stated in the sequence header. It gives you an AVCodecContext with enough parameters for most non decoding scenarios immediately after avcodec_open2(). They will be updated with frame dimensions afterwards if any is parsed. Also, the AVCodecContext dimension fields are nonetheless documented to not necessarily match the latest returned frame's dimensions. Applying this set without this patch, if this is the only decoder available and is therefore used for probing, some of the remuxing FATE tests will fail once a pix_fmt is not assigned and frames are then not parsed. > > - Mark > _______________________________________________ > 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 29/09/2020 17:23, James Almer wrote: > On 9/29/2020 12:52 PM, Mark Thompson wrote: >> On 25/09/2020 15:43, James Almer wrote: >>> Signed-off-by: James Almer <jamrial@gmail.com> >>> --- >>> libavcodec/av1dec.c | 10 ++++++++++ >>> 1 file changed, 10 insertions(+) >>> >>> diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c >>> index 0bb04a3e44..f6b9fbbac3 100644 >>> --- a/libavcodec/av1dec.c >>> +++ b/libavcodec/av1dec.c >>> @@ -405,6 +405,9 @@ static av_cold int av1_decode_free(AVCodecContext >>> *avctx) >>> static int set_context_with_sequence(AVCodecContext *avctx, >>> const AV1RawSequenceHeader *seq) >>> { >>> + int width = seq->max_frame_width_minus_1 + 1; >>> + int height = seq->max_frame_height_minus_1 + 1; >>> + >>> avctx->profile = seq->seq_profile; >>> avctx->level = seq->seq_level_idx[0]; >>> @@ -423,6 +426,13 @@ static int >>> set_context_with_sequence(AVCodecContext *avctx, >>> break; >>> } >>> + if (avctx->width != width || avctx->height != height) { >>> + int ret = ff_set_dimensions(avctx, width, height); >>> + if (ret < 0) >>> + return ret; >>> + } >>> + avctx->sample_aspect_ratio = (AVRational) { 1, 1 }; >>> + >>> if (seq->timing_info.num_units_in_display_tick && >>> seq->timing_info.time_scale) { >>> av_reduce(&avctx->framerate.den, &avctx->framerate.num, >>> >> >> Can you explain more about why this might be wanted? There is no >> requirement that the stream actually contains any frames with the size >> stated in the sequence header. > > It gives you an AVCodecContext with enough parameters for most non > decoding scenarios immediately after avcodec_open2(). They will be > updated with frame dimensions afterwards if any is parsed. > Also, the AVCodecContext dimension fields are nonetheless documented to > not necessarily match the latest returned frame's dimensions. > > Applying this set without this patch, if this is the only decoder > available and is therefore used for probing, some of the remuxing FATE > tests will fail once a pix_fmt is not assigned and frames are then not > parsed. Ok, that seems fair enough. Cases where this number is actually wrong are hopefully rare, anyway. - Mark
On 9/29/2020 3:10 PM, Mark Thompson wrote: > On 29/09/2020 17:23, James Almer wrote: >> On 9/29/2020 12:52 PM, Mark Thompson wrote: >>> On 25/09/2020 15:43, James Almer wrote: >>>> Signed-off-by: James Almer <jamrial@gmail.com> >>>> --- >>>> libavcodec/av1dec.c | 10 ++++++++++ >>>> 1 file changed, 10 insertions(+) >>>> >>>> diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c >>>> index 0bb04a3e44..f6b9fbbac3 100644 >>>> --- a/libavcodec/av1dec.c >>>> +++ b/libavcodec/av1dec.c >>>> @@ -405,6 +405,9 @@ static av_cold int av1_decode_free(AVCodecContext >>>> *avctx) >>>> static int set_context_with_sequence(AVCodecContext *avctx, >>>> const AV1RawSequenceHeader >>>> *seq) >>>> { >>>> + int width = seq->max_frame_width_minus_1 + 1; >>>> + int height = seq->max_frame_height_minus_1 + 1; >>>> + >>>> avctx->profile = seq->seq_profile; >>>> avctx->level = seq->seq_level_idx[0]; >>>> @@ -423,6 +426,13 @@ static int >>>> set_context_with_sequence(AVCodecContext *avctx, >>>> break; >>>> } >>>> + if (avctx->width != width || avctx->height != height) { >>>> + int ret = ff_set_dimensions(avctx, width, height); >>>> + if (ret < 0) >>>> + return ret; >>>> + } >>>> + avctx->sample_aspect_ratio = (AVRational) { 1, 1 }; >>>> + >>>> if (seq->timing_info.num_units_in_display_tick && >>>> seq->timing_info.time_scale) { >>>> av_reduce(&avctx->framerate.den, &avctx->framerate.num, >>>> >>> >>> Can you explain more about why this might be wanted? There is no >>> requirement that the stream actually contains any frames with the size >>> stated in the sequence header. >> >> It gives you an AVCodecContext with enough parameters for most non >> decoding scenarios immediately after avcodec_open2(). They will be >> updated with frame dimensions afterwards if any is parsed. >> Also, the AVCodecContext dimension fields are nonetheless documented to >> not necessarily match the latest returned frame's dimensions. >> >> Applying this set without this patch, if this is the only decoder >> available and is therefore used for probing, some of the remuxing FATE >> tests will fail once a pix_fmt is not assigned and frames are then not >> parsed. > > Ok, that seems fair enough. Cases where this number is actually wrong > are hopefully rare, anyway. > > - Mark Pushed, thanks.
diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c index 0bb04a3e44..f6b9fbbac3 100644 --- a/libavcodec/av1dec.c +++ b/libavcodec/av1dec.c @@ -405,6 +405,9 @@ static av_cold int av1_decode_free(AVCodecContext *avctx) static int set_context_with_sequence(AVCodecContext *avctx, const AV1RawSequenceHeader *seq) { + int width = seq->max_frame_width_minus_1 + 1; + int height = seq->max_frame_height_minus_1 + 1; + avctx->profile = seq->seq_profile; avctx->level = seq->seq_level_idx[0]; @@ -423,6 +426,13 @@ static int set_context_with_sequence(AVCodecContext *avctx, break; } + if (avctx->width != width || avctx->height != height) { + int ret = ff_set_dimensions(avctx, width, height); + if (ret < 0) + return ret; + } + avctx->sample_aspect_ratio = (AVRational) { 1, 1 }; + if (seq->timing_info.num_units_in_display_tick && seq->timing_info.time_scale) { av_reduce(&avctx->framerate.den, &avctx->framerate.num,
Signed-off-by: James Almer <jamrial@gmail.com> --- libavcodec/av1dec.c | 10 ++++++++++ 1 file changed, 10 insertions(+)