| Message ID | 20171213105940.32103-10-onemda@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
On Wed, Dec 13, 2017 at 11:59:26AM +0100, Paul B Mahol wrote: > Signed-off-by: Paul B Mahol <onemda@gmail.com> > --- > libavcodec/mjpegdec.c | 18 +++++++++--------- > libavcodec/tdsc.c | 2 +- > tests/fate/vcodec.mak | 4 ++-- > tests/ref/fate/api-mjpeg-codec-param | 4 ++-- > tests/ref/fate/exif-image-embedded | 2 +- > tests/ref/fate/exif-image-jpg | 2 +- > 6 files changed, 16 insertions(+), 16 deletions(-) this breaks ffplay playing a mjpeg in avi ./ffmpeg -i matrixbench_mpeg2.mpg -vcodec mjpeg -t 0.5 -qscale 1 mjpeg.avi ./ffplay mjpeg.avi the output of ffplay looks darker than it should be [...]
On 12/13/2017 4:22 PM, Michael Niedermayer wrote: > On Wed, Dec 13, 2017 at 11:59:26AM +0100, Paul B Mahol wrote: >> Signed-off-by: Paul B Mahol <onemda@gmail.com> >> --- >> libavcodec/mjpegdec.c | 18 +++++++++--------- >> libavcodec/tdsc.c | 2 +- >> tests/fate/vcodec.mak | 4 ++-- >> tests/ref/fate/api-mjpeg-codec-param | 4 ++-- >> tests/ref/fate/exif-image-embedded | 2 +- >> tests/ref/fate/exif-image-jpg | 2 +- >> 6 files changed, 16 insertions(+), 16 deletions(-) > > this breaks ffplay playing a mjpeg in avi > > ./ffmpeg -i matrixbench_mpeg2.mpg -vcodec mjpeg -t 0.5 -qscale 1 mjpeg.avi > ./ffplay mjpeg.avi > > the output of ffplay looks darker than it should be Without this patch swscale emits a warning about deprecated pixel format. With it, the warning is gone. Guess swscale needs to be adapted in some way before the same is done to the decoders?
On Wed, 13 Dec 2017, Michael Niedermayer wrote: > On Wed, Dec 13, 2017 at 11:59:26AM +0100, Paul B Mahol wrote: >> Signed-off-by: Paul B Mahol <onemda@gmail.com> >> --- >> libavcodec/mjpegdec.c | 18 +++++++++--------- >> libavcodec/tdsc.c | 2 +- >> tests/fate/vcodec.mak | 4 ++-- >> tests/ref/fate/api-mjpeg-codec-param | 4 ++-- >> tests/ref/fate/exif-image-embedded | 2 +- >> tests/ref/fate/exif-image-jpg | 2 +- >> 6 files changed, 16 insertions(+), 16 deletions(-) > > this breaks ffplay playing a mjpeg in avi > > ./ffmpeg -i matrixbench_mpeg2.mpg -vcodec mjpeg -t 0.5 -qscale 1 mjpeg.avi > ./ffplay mjpeg.avi > > the output of ffplay looks darker than it should be FFplay does not specify the needed range for its buffersink. If there is a way to specify allowed combinations (e.g. YUV+limited, YUV+unspecified, RGB+full, RGB+unspecified), then this probably can be fixed. (As far as I know SDL also does not specify the range of the used pixel formats, but I think YUV is always limited range there, and RGB is always full range) Regards, Marton
On Wed, Dec 13, 2017 at 04:44:26PM -0300, James Almer wrote: > On 12/13/2017 4:22 PM, Michael Niedermayer wrote: > > On Wed, Dec 13, 2017 at 11:59:26AM +0100, Paul B Mahol wrote: > >> Signed-off-by: Paul B Mahol <onemda@gmail.com> > >> --- > >> libavcodec/mjpegdec.c | 18 +++++++++--------- > >> libavcodec/tdsc.c | 2 +- > >> tests/fate/vcodec.mak | 4 ++-- > >> tests/ref/fate/api-mjpeg-codec-param | 4 ++-- > >> tests/ref/fate/exif-image-embedded | 2 +- > >> tests/ref/fate/exif-image-jpg | 2 +- > >> 6 files changed, 16 insertions(+), 16 deletions(-) > > > > this breaks ffplay playing a mjpeg in avi > > > > ./ffmpeg -i matrixbench_mpeg2.mpg -vcodec mjpeg -t 0.5 -qscale 1 mjpeg.avi > > ./ffplay mjpeg.avi > > > > the output of ffplay looks darker than it should be > > Without this patch swscale emits a warning about deprecated pixel > format. With it, the warning is gone. > > Guess swscale needs to be adapted in some way before the same is done to > the decoders? To clarify the report, the mjpeg.avi is the same before and after the patch, the difference is in the ffplay command [...]
On Wed, 13 Dec 2017 20:56:30 +0100 (CET) Marton Balint <cus@passwd.hu> wrote: > On Wed, 13 Dec 2017, Michael Niedermayer wrote: > > > On Wed, Dec 13, 2017 at 11:59:26AM +0100, Paul B Mahol wrote: > >> Signed-off-by: Paul B Mahol <onemda@gmail.com> > >> --- > >> libavcodec/mjpegdec.c | 18 +++++++++--------- > >> libavcodec/tdsc.c | 2 +- > >> tests/fate/vcodec.mak | 4 ++-- > >> tests/ref/fate/api-mjpeg-codec-param | 4 ++-- > >> tests/ref/fate/exif-image-embedded | 2 +- > >> tests/ref/fate/exif-image-jpg | 2 +- > >> 6 files changed, 16 insertions(+), 16 deletions(-) > > > > this breaks ffplay playing a mjpeg in avi > > > > ./ffmpeg -i matrixbench_mpeg2.mpg -vcodec mjpeg -t 0.5 -qscale 1 mjpeg.avi > > ./ffplay mjpeg.avi > > > > the output of ffplay looks darker than it should be > > FFplay does not specify the needed range for its buffersink. If there is a > way to specify allowed combinations (e.g. YUV+limited, YUV+unspecified, > RGB+full, RGB+unspecified), then this probably can be fixed. > > (As far as I know SDL also does not specify the range of the used > pixel formats, but I think YUV is always limited range there, and > RGB is always full range) If a lavfi API user suddenly has to specify the range manually (instead just over AVFrame), then this is a compatibility issue too. (I'm talking about non-J pixfmts in both cases.) Poor Paul...
On 12/14/17, wm4 <nfxjfg@googlemail.com> wrote: > On Wed, 13 Dec 2017 20:56:30 +0100 (CET) > Marton Balint <cus@passwd.hu> wrote: > >> On Wed, 13 Dec 2017, Michael Niedermayer wrote: >> >> > On Wed, Dec 13, 2017 at 11:59:26AM +0100, Paul B Mahol wrote: >> >> Signed-off-by: Paul B Mahol <onemda@gmail.com> >> >> --- >> >> libavcodec/mjpegdec.c | 18 +++++++++--------- >> >> libavcodec/tdsc.c | 2 +- >> >> tests/fate/vcodec.mak | 4 ++-- >> >> tests/ref/fate/api-mjpeg-codec-param | 4 ++-- >> >> tests/ref/fate/exif-image-embedded | 2 +- >> >> tests/ref/fate/exif-image-jpg | 2 +- >> >> 6 files changed, 16 insertions(+), 16 deletions(-) >> > >> > this breaks ffplay playing a mjpeg in avi >> > >> > ./ffmpeg -i matrixbench_mpeg2.mpg -vcodec mjpeg -t 0.5 -qscale 1 >> > mjpeg.avi >> > ./ffplay mjpeg.avi >> > >> > the output of ffplay looks darker than it should be >> >> FFplay does not specify the needed range for its buffersink. If there is a >> >> way to specify allowed combinations (e.g. YUV+limited, YUV+unspecified, >> RGB+full, RGB+unspecified), then this probably can be fixed. >> >> (As far as I know SDL also does not specify the range of the used >> pixel formats, but I think YUV is always limited range there, and >> RGB is always full range) > > If a lavfi API user suddenly has to specify the range manually (instead > just over AVFrame), then this is a compatibility issue too. (I'm > talking about non-J pixfmts in both cases.) Poor Paul... color range via api, either buffersink or buffersrc are optional and not mandatory. It is ffplay issue that it ignores color range, comming from AVFrame itself.
diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c index e005dd0cd3..55676d8576 100644 --- a/libavcodec/mjpegdec.c +++ b/libavcodec/mjpegdec.c @@ -467,7 +467,7 @@ int ff_mjpeg_decode_sof(MJpegDecodeContext *s) if (s->component_id[0] == 'Q' && s->component_id[1] == 'F' && s->component_id[2] == 'A') { s->avctx->pix_fmt = s->bits <= 8 ? AV_PIX_FMT_GBRP : AV_PIX_FMT_GBRP16; } else { - if (s->bits <= 8) s->avctx->pix_fmt = s->cs_itu601 ? AV_PIX_FMT_YUV444P : AV_PIX_FMT_YUVJ444P; + if (s->bits <= 8) s->avctx->pix_fmt = AV_PIX_FMT_YUV444P; else s->avctx->pix_fmt = AV_PIX_FMT_YUV444P16; s->avctx->color_range = s->cs_itu601 ? AVCOL_RANGE_MPEG : AVCOL_RANGE_JPEG; } @@ -509,7 +509,7 @@ int ff_mjpeg_decode_sof(MJpegDecodeContext *s) case 0x22122100: case 0x21211100: case 0x22211200: - if (s->bits <= 8) s->avctx->pix_fmt = s->cs_itu601 ? AV_PIX_FMT_YUV444P : AV_PIX_FMT_YUVJ444P; + if (s->bits <= 8) s->avctx->pix_fmt = AV_PIX_FMT_YUV444P; else goto unk_pixfmt; s->avctx->color_range = s->cs_itu601 ? AVCOL_RANGE_MPEG : AVCOL_RANGE_JPEG; @@ -517,7 +517,7 @@ int ff_mjpeg_decode_sof(MJpegDecodeContext *s) case 0x22221100: case 0x22112200: case 0x11222200: - if (s->bits <= 8) s->avctx->pix_fmt = s->cs_itu601 ? AV_PIX_FMT_YUV444P : AV_PIX_FMT_YUVJ444P; + if (s->bits <= 8) s->avctx->pix_fmt = AV_PIX_FMT_YUV444P; else goto unk_pixfmt; s->avctx->color_range = s->cs_itu601 ? AVCOL_RANGE_MPEG : AVCOL_RANGE_JPEG; @@ -549,7 +549,7 @@ int ff_mjpeg_decode_sof(MJpegDecodeContext *s) } else { if (pix_fmt_id == 0x14111100) s->upscale_v[1] = s->upscale_v[2] = 1; - if (s->bits <= 8) s->avctx->pix_fmt = s->cs_itu601 ? AV_PIX_FMT_YUV440P : AV_PIX_FMT_YUVJ440P; + if (s->bits <= 8) s->avctx->pix_fmt = AV_PIX_FMT_YUV440P; else goto unk_pixfmt; s->avctx->color_range = s->cs_itu601 ? AVCOL_RANGE_MPEG : AVCOL_RANGE_JPEG; @@ -562,7 +562,7 @@ int ff_mjpeg_decode_sof(MJpegDecodeContext *s) goto unk_pixfmt; s->upscale_h[0] = s->upscale_h[1] = 1; } else { - if (s->bits <= 8) s->avctx->pix_fmt = s->cs_itu601 ? AV_PIX_FMT_YUV422P : AV_PIX_FMT_YUVJ422P; + if (s->bits <= 8) s->avctx->pix_fmt = AV_PIX_FMT_YUV422P; else s->avctx->pix_fmt = AV_PIX_FMT_YUV422P16; s->avctx->color_range = s->cs_itu601 ? AVCOL_RANGE_MPEG : AVCOL_RANGE_JPEG; } @@ -570,13 +570,13 @@ int ff_mjpeg_decode_sof(MJpegDecodeContext *s) case 0x31111100: if (s->bits > 8) goto unk_pixfmt; - s->avctx->pix_fmt = s->cs_itu601 ? AV_PIX_FMT_YUV444P : AV_PIX_FMT_YUVJ444P; + s->avctx->pix_fmt = AV_PIX_FMT_YUV444P; s->avctx->color_range = s->cs_itu601 ? AVCOL_RANGE_MPEG : AVCOL_RANGE_JPEG; s->upscale_h[1] = s->upscale_h[2] = 2; break; case 0x22121100: case 0x22111200: - if (s->bits <= 8) s->avctx->pix_fmt = s->cs_itu601 ? AV_PIX_FMT_YUV422P : AV_PIX_FMT_YUVJ422P; + if (s->bits <= 8) s->avctx->pix_fmt = AV_PIX_FMT_YUV422P; else goto unk_pixfmt; s->avctx->color_range = s->cs_itu601 ? AVCOL_RANGE_MPEG : AVCOL_RANGE_JPEG; @@ -584,7 +584,7 @@ int ff_mjpeg_decode_sof(MJpegDecodeContext *s) case 0x22111100: case 0x42111100: case 0x24111100: - if (s->bits <= 8) s->avctx->pix_fmt = s->cs_itu601 ? AV_PIX_FMT_YUV420P : AV_PIX_FMT_YUVJ420P; + if (s->bits <= 8) s->avctx->pix_fmt = AV_PIX_FMT_YUV420P; else s->avctx->pix_fmt = AV_PIX_FMT_YUV420P16; s->avctx->color_range = s->cs_itu601 ? AVCOL_RANGE_MPEG : AVCOL_RANGE_JPEG; if (pix_fmt_id == 0x42111100) { @@ -598,7 +598,7 @@ int ff_mjpeg_decode_sof(MJpegDecodeContext *s) } break; case 0x41111100: - if (s->bits <= 8) s->avctx->pix_fmt = s->cs_itu601 ? AV_PIX_FMT_YUV411P : AV_PIX_FMT_YUVJ411P; + if (s->bits <= 8) s->avctx->pix_fmt = AV_PIX_FMT_YUV411P; else goto unk_pixfmt; s->avctx->color_range = s->cs_itu601 ? AVCOL_RANGE_MPEG : AVCOL_RANGE_JPEG; diff --git a/libavcodec/tdsc.c b/libavcodec/tdsc.c index 4182404cf0..af92ef6ccc 100644 --- a/libavcodec/tdsc.c +++ b/libavcodec/tdsc.c @@ -357,7 +357,7 @@ static int tdsc_decode_jpeg_tile(AVCodecContext *avctx, int tile_size, } ret = avcodec_receive_frame(ctx->jpeg_avctx, ctx->jpgframe); - if (ret < 0 || ctx->jpgframe->format != AV_PIX_FMT_YUVJ420P) { + if (ret < 0 || ctx->jpgframe->format != AV_PIX_FMT_YUV420P) { av_log(avctx, AV_LOG_ERROR, "JPEG decoding error (%d).\n", ret); diff --git a/tests/fate/vcodec.mak b/tests/fate/vcodec.mak index bbcf25d72a..72fc46e83b 100644 --- a/tests/fate/vcodec.mak +++ b/tests/fate/vcodec.mak @@ -4,8 +4,8 @@ fate-vsynth_lena-%: SRC = tests/data/vsynth_lena.yuv fate-vsynth3-%: SRC = tests/data/vsynth3.yuv fate-vsynth%: CODEC = $(word 3, $(subst -, ,$(@))) fate-vsynth%: FMT = avi -fate-vsynth%: CMD = enc_dec "rawvideo -s 352x288 -pix_fmt yuv420p $(RAWDECOPTS)" $(SRC) $(FMT) "-c $(CODEC) $(ENCOPTS)" rawvideo "-s 352x288 -pix_fmt yuv420p -vsync 0 $(DECOPTS)" -keep "$(DECINOPTS)" -fate-vsynth3-%: CMD = enc_dec "rawvideo -s $(FATEW)x$(FATEH) -pix_fmt yuv420p $(RAWDECOPTS)" $(SRC) $(FMT) "-c $(CODEC) $(ENCOPTS)" rawvideo "-s $(FATEW)x$(FATEH) -pix_fmt yuv420p -vsync 0 $(DECOPTS)" -keep "$(DECINOPTS)" +fate-vsynth%: CMD = enc_dec "rawvideo -s 352x288 -color_range tv -pix_fmt yuv420p $(RAWDECOPTS)" $(SRC) $(FMT) "-c $(CODEC) $(ENCOPTS)" rawvideo "-s 352x288 -pix_fmt yuv420p -color_range tv -vsync 0 $(DECOPTS)" -keep "$(DECINOPTS)" +fate-vsynth3-%: CMD = enc_dec "rawvideo -s $(FATEW)x$(FATEH) -color_range tv -pix_fmt yuv420p $(RAWDECOPTS)" $(SRC) $(FMT) "-c $(CODEC) $(ENCOPTS)" rawvideo "-s $(FATEW)x$(FATEH) -pix_fmt yuv420p -color_range tv -vsync 0 $(DECOPTS)" -keep "$(DECINOPTS)" fate-vsynth%: CMP_UNIT = 1 fate-vsynth%: REF = $(SRC_PATH)/tests/ref/vsynth/$(@:fate-%=%) diff --git a/tests/ref/fate/api-mjpeg-codec-param b/tests/ref/fate/api-mjpeg-codec-param index 178b7c73cb..0ef9594b40 100644 --- a/tests/ref/fate/api-mjpeg-codec-param +++ b/tests/ref/fate/api-mjpeg-codec-param @@ -133,7 +133,7 @@ stream=0, decode=0 field_order=0 dump_separator= codec_whitelist= - pixel_format=yuvj422p + pixel_format=yuv422p video_size=400x225 max_pixels=2147483647 hwaccel_flags=0x00000001 @@ -272,7 +272,7 @@ stream=0, decode=1 field_order=0 dump_separator= codec_whitelist= - pixel_format=yuvj422p + pixel_format=yuv422p video_size=400x225 max_pixels=2147483647 hwaccel_flags=0x00000001 diff --git a/tests/ref/fate/exif-image-embedded b/tests/ref/fate/exif-image-embedded index 306ae0854b..582f09dfb6 100644 --- a/tests/ref/fate/exif-image-embedded +++ b/tests/ref/fate/exif-image-embedded @@ -14,7 +14,7 @@ pkt_pos=N/A pkt_size=15760 width=263 height=263 -pix_fmt=yuvj420p +pix_fmt=yuv420p sample_aspect_ratio=1:1 pict_type=I coded_picture_number=0 diff --git a/tests/ref/fate/exif-image-jpg b/tests/ref/fate/exif-image-jpg index b266501191..6b3dbb3319 100644 --- a/tests/ref/fate/exif-image-jpg +++ b/tests/ref/fate/exif-image-jpg @@ -14,7 +14,7 @@ pkt_pos=N/A pkt_size=46095 width=400 height=225 -pix_fmt=yuvj422p +pix_fmt=yuv422p sample_aspect_ratio=1:1 pict_type=I coded_picture_number=0
Signed-off-by: Paul B Mahol <onemda@gmail.com> --- libavcodec/mjpegdec.c | 18 +++++++++--------- libavcodec/tdsc.c | 2 +- tests/fate/vcodec.mak | 4 ++-- tests/ref/fate/api-mjpeg-codec-param | 4 ++-- tests/ref/fate/exif-image-embedded | 2 +- tests/ref/fate/exif-image-jpg | 2 +- 6 files changed, 16 insertions(+), 16 deletions(-)