On Sun, Mar 29, 2015 at 7:39 PM, Anton Khirnov <[email protected]> wrote:
> Quoting Diego Biurrun (2015-03-29 19:10:09)
>> On Sun, Mar 29, 2015 at 03:09:53PM +0200, Anton Khirnov wrote:
>> > Quoting Diego Biurrun (2015-03-29 14:48:58)
>> > > On Sun, Mar 29, 2015 at 02:18:54PM +0200, Anton Khirnov wrote:
>> > > > --- a/libavcodec/hevc.c
>> > > > +++ b/libavcodec/hevc.c
>> > > > @@ -383,24 +383,63 @@ static int decode_lt_rps(HEVCContext *s, 
>> > > > LongTermRPS *rps, GetBitContext *gb)
>> > > > +    avctx->pix_fmt             = sps->pix_fmt;
>> > > > +    avctx->coded_width         = sps->width;
>> > > > +    avctx->coded_height        = sps->height;
>> > > > +    avctx->width               = sps->output_width;
>> > > > +    avctx->height              = sps->output_height;
>> > > > +    avctx->has_b_frames        = 
>> > > > sps->temporal_layer[sps->max_sub_layers - 1].num_reorder_pics;
>> > > > +
>> > > > -    s->avctx->coded_width         = sps->width;
>> > > > -    s->avctx->coded_height        = sps->height;
>> > > > -    s->avctx->width               = sps->output_width;
>> > > > -    s->avctx->height              = sps->output_height;
>> > > > -    s->avctx->has_b_frames        = 
>> > > > sps->temporal_layer[sps->max_sub_layers - 1].num_reorder_pics;
>> > >
>> > > Is there a reason to suddenly set pix_fmt as well?  It sure looks odd in
>> > > a refactoring-only patch.
>> >
>> > Yes, there is.
>> > Currently, that function sets pixel format taking into account hwaccels,
>> > but that should not be done during init.
>>
>> I still don't understand.  It's not done during init AFAICT.  You're doing
>> it in the same place now.  Why wasn't it needed before and why doesn't the
>> log message explain it?
>>
>
> Ok, more slowly.
>
> Currently, set_sps() obtains the pixel format by calling
> ff_get_format(), which might call a user callback to initialize hw
> acceleration. And currently we do NOT export any stream parameters
> during init.
>
> Now, we want to export the parameters we got from extradata during init.
> To avoid duplication, we want to use the same code. But we do not want
> to call ff_get_format() yet, we just want to export the underlying sw
> pixel format.
>
> To accomplish that, I add a new line that does just this -- export the
> software pixel format to AVCodecContext. It's useless when this new
> function is called from set_sps(), since it will be immediately
> overwritten by the result of ff_get_format, but it's still better to set
> it here rather than scatter such code over the file.

Would it be possible to have this explanation in the commit log?
I think it should be fine anyway.
-- 
Vittorio
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to