[FFmpeg-devel] [PATCH] sbcdec: do not unnecessarily set frame properties

[Michael Niedermayer]

On Wed, Mar 18, 2020 at 09:35:56AM +0100, Anton Khirnov wrote:
> Quoting Michael Niedermayer (2020-03-16 21:58:52)
> > On Wed, Jan 29, 2020 at 06:00:16PM +0100, Anton Khirnov wrote:
> > > Decoders are supposed to export stream properties in AVCodecContext, the
> > > AVFrame properties are set from those in ff_get_buffer().
> > > ---
> > >  libavcodec/sbcdec.c | 7 ++++---
> > >  1 file changed, 4 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/libavcodec/sbcdec.c b/libavcodec/sbcdec.c
> > > index d8ea6855fe..2ebde46627 100644
> > > --- a/libavcodec/sbcdec.c
> > > +++ b/libavcodec/sbcdec.c
> > > @@ -324,6 +324,8 @@ static int sbc_decode_init(AVCodecContext *avctx)
> > >      SBCDecContext *sbc = avctx->priv_data;
> > >      int i, ch;
> > >  
> > > +    avctx->sample_fmt = AV_SAMPLE_FMT_S16P;
> > > +
> > >      sbc->frame.crc_ctx = av_crc_get_table(AV_CRC_8_EBU);
> > >  
> > >      memset(sbc->dsp.V, 0, sizeof(sbc->dsp.V));
> > > @@ -348,9 +350,8 @@ static int sbc_decode_frame(AVCodecContext *avctx,
> > >      if (frame_length <= 0)
> > >          return frame_length;
> > >  
> > > -    avctx->channels =
> > > -    frame->channels = sbc->frame.channels;
> > > -    frame->format = AV_SAMPLE_FMT_S16P;
> > > +    avctx->channels = sbc->frame.channels;
> > > +
> > 
> > probably ok but the design of exporting data which describes the current frame
> > in the main context instead of the frames context. It gives a moment pause
> > not feeling that this is the ideal design
> 
> Perhaps, but that's how things evolved historically and how things now
> are. One decoder behaving differently from all the others is confusing,
> especially when there is no reason for it.
> 
> FWIW I would be totally in favor of reducing the use of AVCodecContext
> variables (it's too much of a "god structure") and switch to using frame
> variables for this. But it's a lot of boring work and someone would have
> to do it.

i agree with everything you said above

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Never trust a computer, one day, it may think you are the virus. -- Compn
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200318/dac2b2f8/attachment.sig>