Re: [FFmpeg-devel] [PATCH 1/3] lavc/mjpegdec: add function ff_mjpeg_decode_header
> 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
>
> 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
> > ---
> > 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".
Re: [FFmpeg-devel] [PATCH 1/3] lavc/mjpegdec: add function ff_mjpeg_decode_header
>Sorry for not realizing this earlier (I searched for "SOF0"): >Why is this function duplicated? > >Carl Eugen Hi Carl: You can find the difference: here I just find frame header markers (SOF0 ~ SOF 3), mjpeg decoder try to find all markers. Thanks Zhong ___ 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".
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
> ---
> 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
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Whats the most studid thing your enemy could do ? Blow himself up
Whats the most studid thing you could do ? Give up your rights and
freedom because your enemy blew himself up.
signature.asc
Description: PGP signature
___
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".
Re: [FFmpeg-devel] [PATCH 1/3] lavc/mjpegdec: add function ff_mjpeg_decode_header
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Carl Eugen Hoyos
> Sent: Friday, June 28, 2019 2:56 AM
> To: FFmpeg development discussions and patches
>
> Subject: Re: [FFmpeg-devel] [PATCH 1/3] lavc/mjpegdec: add function
> ff_mjpeg_decode_header
>
> Am Do., 27. Juni 2019 um 14:59 Uhr schrieb Zhong Li :
>
> > -if (s->avctx->pix_fmt == s->hwaccel_sw_pix_fmt
> && !size_change) {
> > +if (!(s->got_picture && s->interlaced && (s->bottom_field
> == !s->interlace_polarity))) {
> > +if (s->avctx->pix_fmt == s->hwaccel_sw_pix_fmt &&
> > + !s->size_change) {
>
> Is this an (unrelated) bug fix or only vaapi-related?
> I wonder if it should be in this patch for both cases.
Hi Carl:
This is not to fix any issue, just a tailing after refact with
ff_mjpeg_decode_header():
Original code:
if (s->got_picture && s->interlaced && (s->bottom_field ==
!s->interlace_polarity)) {
if (s->progressive) {
avpriv_request_sample(s->avctx, "progressively coded interlaced
picture");
return AVERROR_INVALIDDATA;
}
} else {
...
if (s->avctx->pix_fmt == s->hwaccel_sw_pix_fmt && !size_change) {
s->avctx->pix_fmt = s->hwaccel_pix_fmt;
} else {
enum AVPixelFormat pix_fmts[] = {
#if CONFIG_MJPEG_NVDEC_HWACCEL
AV_PIX_FMT_CUDA,
#endif
#if CONFIG_MJPEG_VAAPI_HWACCEL
AV_PIX_FMT_VAAPI,
#endif
s->avctx->pix_fmt,
AV_PIX_FMT_NONE,
};
s->hwaccel_pix_fmt = ff_get_format(s->avctx, pix_fmts);
...
}
Thanks
Zhong
___
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".
Re: [FFmpeg-devel] [PATCH 1/3] lavc/mjpegdec: add function ff_mjpeg_decode_header
Am Do., 27. Juni 2019 um 14:59 Uhr schrieb Zhong Li :
> -if (s->avctx->pix_fmt == s->hwaccel_sw_pix_fmt && !size_change) {
> +if (!(s->got_picture && s->interlaced && (s->bottom_field ==
> !s->interlace_polarity))) {
> +if (s->avctx->pix_fmt == s->hwaccel_sw_pix_fmt && !s->size_change) {
Is this an (unrelated) bug fix or only vaapi-related?
I wonder if it should be in this patch for both cases.
> s->avctx->pix_fmt = s->hwaccel_pix_fmt;
> +s->size_change = 0;
Thank you, Carl Eugen
___
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".
Re: [FFmpeg-devel] [PATCH 1/3] lavc/mjpegdec: add function ff_mjpeg_decode_header
> From: Li, Zhong > Sent: Thursday, June 27, 2019 8:59 PM > To: ffmpeg-devel@ffmpeg.org > Cc: Li, Zhong > Subject: [PATCH 1/3] lavc/mjpegdec: add function ff_mjpeg_decode_header > > It will be reused in the following mjpeg_parser patch > > Signed-off-by: Zhong Li > --- > 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. Sorry, this annotation should be part of patch 3/3. Please ignore here. ___ 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".
