> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Michael Niedermayer
> Sent: Friday, June 28, 2019 8:52 PM
> To: FFmpeg development discussions and patches
> <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 1/3] lavc/mjpegdec: add function
> ff_mjpeg_decode_header
>
> On Thu, Jun 27, 2019 at 08:59:12PM +0800, Zhong Li wrote:
> > It will be reused in the following mjpeg_parser patch
> >
> > Signed-off-by: Zhong Li <zhong...@intel.com>
> > ---
> > Mark Thompson: This seems suspicious - MJPEG is generally 4:2:2 (e.g.
> > UVC requires it), so I would expect a 4:2:2 format to be the default
> > here? (Or maybe a longer list - VAAPI certainly supports 4:2:2, 4:2:0
> > and 4:4:4 on the same hardware.)
> > Zhong: libmfx can support jpeg baseline profile with more output formats,
> but current ffmpeg-qsv decoder/vpp can't. Will extend supported format list
> as separated patch.
> >
> > libavcodec/mjpegdec.c | 37 ++++++++++++++++++++++++++++---------
> > libavcodec/mjpegdec.h | 4 ++++
> > 2 files changed, 32 insertions(+), 9 deletions(-)
> >
> > diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c index
> > a65bc8d..5da66bb 100644
> > --- a/libavcodec/mjpegdec.c
> > +++ b/libavcodec/mjpegdec.c
> > @@ -157,6 +157,8 @@ av_cold int
> ff_mjpeg_decode_init(AVCodecContext *avctx)
> > s->start_code = -1;
> > s->first_picture = 1;
> > s->got_picture = 0;
> > + s->reinit_idct = 0;
> > + s->size_change = 0;
> > s->org_height = avctx->coded_height;
> > avctx->chroma_sample_location = AVCHROMA_LOC_CENTER;
> > avctx->colorspace = AVCOL_SPC_BT470BG; @@ -302,9 +304,9 @@
> int
> > ff_mjpeg_decode_dht(MJpegDecodeContext *s)
> > return 0;
> > }
> >
> > -int ff_mjpeg_decode_sof(MJpegDecodeContext *s)
> > +int ff_mjpeg_decode_header(MJpegDecodeContext *s)
> > {
> > - int len, nb_components, i, width, height, bits, ret, size_change;
> > + int len, nb_components, i, width, height, bits, ret;
> > unsigned pix_fmt_id;
> > int h_count[MAX_COMPONENTS] = { 0 };
> > int v_count[MAX_COMPONENTS] = { 0 }; @@ -324,7 +326,7 @@ int
> > ff_mjpeg_decode_sof(MJpegDecodeContext *s)
> > if (s->avctx->bits_per_raw_sample != bits) {
> > av_log(s->avctx, s->avctx->bits_per_raw_sample > 0 ?
> AV_LOG_INFO : AV_LOG_DEBUG, "Changing bps from %d to %d\n",
> s->avctx->bits_per_raw_sample, bits);
> > s->avctx->bits_per_raw_sample = bits;
> > - init_idct(s->avctx);
> > + s->reinit_idct = 1;
> > }
>
> I think the avctx->bits_per_raw_sample value should stay in sync with the
> initialized idct
>
> This patch would allow the bits_per_raw_sample to change and then a
> subsequent error to skip init_idct() maybe this is ok as it would be probably
> called later in a subsequent frame but i think its better if they stay closer
> together or there is nothing between them that can cause ine to exeucute
> without the other
Thanks for detail comment, actually this is an intended way to resolve a
dependency:
Calling init_idct requires the decoder has been initialized.
static void init_idct(AVCodecContext *avctx)
{
MJpegDecodeContext *s = avctx->priv_data;
...
}
But I hope ff_mjpeg_decode_header can be used for mjpeg_parser, if don't use
current way, will cause segment fault when call init_idct() due to
avctx->priv_data is NULL.
Probably we can change the definition of init_idct(AVCodecContext *avctx) to be
init_idct(MJpegDecodeContext *s) ? (But init_idct is useless for parser).
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
