| Message ID | 20180501194013.9552-12-onemda@gmail.com |
|---|---|
| State | New |
| Headers | show |
Hi On Tue, May 01, 2018 at 09:40:01PM +0200, Paul B Mahol wrote: > Signed-off-by: Paul B Mahol <onemda@gmail.com> > --- > libavcodec/mjpegenc.c | 5 ++++- > libavcodec/mpegvideo_enc.c | 3 +-- > tests/lavf-regression.sh | 2 +- > 3 files changed, 6 insertions(+), 4 deletions(-) This doesnt work first create a input file ./ffmpeg -f lavfi -i testsrc=640x480 test.ppm (if you like check that this has correct white and black) 0,0,0 and 255,255,255 now convert to jpeg ./ffmpeg -i test.ppm test.jpg gimp test.jpg white and black have RGB values: 16,16,16 and 235,235,235 thats not correct and also looks wrong the strict_std_compliance check that was before this cannot be just droped If you want jpeg images to interoperate with all software they must be full range the limited range is a not entirely official feature. So it should not be used by default (that is AFAIK, things may have changed and maybe theres a official way now but from what i remember there was not for images) [...]
On 5/2/18, Michael Niedermayer <michael@niedermayer.cc> wrote: > Hi > > On Tue, May 01, 2018 at 09:40:01PM +0200, Paul B Mahol wrote: >> Signed-off-by: Paul B Mahol <onemda@gmail.com> >> --- >> libavcodec/mjpegenc.c | 5 ++++- >> libavcodec/mpegvideo_enc.c | 3 +-- >> tests/lavf-regression.sh | 2 +- >> 3 files changed, 6 insertions(+), 4 deletions(-) > > This doesnt work It does work, if you want full range set full range as option. Check visually colors. This happens because rgb is still marked incorrectly as limited range, so limited is picked instead. > > first create a input file > ./ffmpeg -f lavfi -i testsrc=640x480 test.ppm > (if you like check that this has correct white and black) 0,0,0 and > 255,255,255 > > now convert to jpeg > ./ffmpeg -i test.ppm test.jpg > gimp test.jpg > > white and black have RGB values: 16,16,16 and 235,235,235 thats not correct > and also looks wrong > > the strict_std_compliance check that was before this cannot be just droped > > If you want jpeg images to interoperate with all software they must be full > > range > the limited range is a not entirely official feature. So it should not be > used by default > (that is AFAIK, things may have changed and maybe theres a official way now > but from what i remember there was not for images) Better check actual paper.
On 1 May 2018 at 23:22, Paul B Mahol <onemda@gmail.com> wrote: > On 5/2/18, Michael Niedermayer <michael@niedermayer.cc> wrote: > > Hi > > > > On Tue, May 01, 2018 at 09:40:01PM +0200, Paul B Mahol wrote: > >> Signed-off-by: Paul B Mahol <onemda@gmail.com> > >> --- > >> libavcodec/mjpegenc.c | 5 ++++- > >> libavcodec/mpegvideo_enc.c | 3 +-- > >> tests/lavf-regression.sh | 2 +- > >> 3 files changed, 6 insertions(+), 4 deletions(-) > > > > This doesnt work > > It does work, if you want full range set full range as option. > Check visually colors. > This happens because rgb is still marked incorrectly as limited range, > so limited is picked instead. > Shouldn't this be part of the patchset?
On Wed, May 02, 2018 at 12:22:10AM +0200, Paul B Mahol wrote: > On 5/2/18, Michael Niedermayer <michael@niedermayer.cc> wrote: > > Hi > > > > On Tue, May 01, 2018 at 09:40:01PM +0200, Paul B Mahol wrote: > >> Signed-off-by: Paul B Mahol <onemda@gmail.com> > >> --- > >> libavcodec/mjpegenc.c | 5 ++++- > >> libavcodec/mpegvideo_enc.c | 3 +-- > >> tests/lavf-regression.sh | 2 +- > >> 3 files changed, 6 insertions(+), 4 deletions(-) > > > > This doesnt work > > It does work, if you want full range set full range as option. > Check visually colors. They vissually look wrong too same as the gimp color picker would suggest > This happens because rgb is still marked incorrectly as limited range, > so limited is picked instead. no, that is not the problem. We could take a mpeg2 file as input which is limited range "always" that takes rgb completely out of the picture and you still should have a jpeg file that will look wrong vissually in many jpeg decoders. libavcodec supports the inofficial limited range extension so that will look correct. > > > > > first create a input file > > ./ffmpeg -f lavfi -i testsrc=640x480 test.ppm > > (if you like check that this has correct white and black) 0,0,0 and > > 255,255,255 > > > > now convert to jpeg > > ./ffmpeg -i test.ppm test.jpg > > gimp test.jpg > > > > white and black have RGB values: 16,16,16 and 235,235,235 thats not correct > > and also looks wrong > > > > the strict_std_compliance check that was before this cannot be just droped > > > > If you want jpeg images to interoperate with all software they must be full > > > > range > > the limited range is a not entirely official feature. So it should not be > > used by default > > (that is AFAIK, things may have changed and maybe theres a official way now > > but from what i remember there was not for images) > > Better check actual paper. I think whoever changes what the encoder outputs should check it its JFIF 1.02 for images. See "Conversion to and from RGB" "Y, Cb, and Cr are converted from R, G, and B as defined in CCIR Recommendation 601 but are normalized so as to occupy the full 256 levels of a 8-bit binary encoding." [...]
On Tue, 1 May 2018, Rostislav Pehlivanov wrote: > On 1 May 2018 at 23:22, Paul B Mahol <onemda@gmail.com> wrote: > >> On 5/2/18, Michael Niedermayer <michael@niedermayer.cc> wrote: >> > Hi >> > >> > On Tue, May 01, 2018 at 09:40:01PM +0200, Paul B Mahol wrote: >> >> Signed-off-by: Paul B Mahol <onemda@gmail.com> >> >> --- >> >> libavcodec/mjpegenc.c | 5 ++++- >> >> libavcodec/mpegvideo_enc.c | 3 +-- >> >> tests/lavf-regression.sh | 2 +- >> >> 3 files changed, 6 insertions(+), 4 deletions(-) >> > >> > This doesnt work >> >> It does work, if you want full range set full range as option. >> Check visually colors. >> This happens because rgb is still marked incorrectly as limited range, >> so limited is picked instead. >> > > Shouldn't this be part of the patchset? I thought that color_range only matters for YUV formats, for RGB it is ignored. If it was decided to support limited RGB, then please update the docs of AVCOL_RANGE_JPEG/MPEG. Otherwise the color_range of an RGB format should not matter. On the other hand, if we decide to support limited RGB, then maybe a new member in color_range is better, this way we are able to specify more easily that we support limited YUV and full range RGB. (like ffplay typically does). Thanks, Marton
On 2 May 2018 at 19:04, Marton Balint <cus@passwd.hu> wrote: > > > On Tue, 1 May 2018, Rostislav Pehlivanov wrote: > > On 1 May 2018 at 23:22, Paul B Mahol <onemda@gmail.com> wrote: >> >> On 5/2/18, Michael Niedermayer <michael@niedermayer.cc> wrote: >>> > Hi >>> > >>> > On Tue, May 01, 2018 at 09:40:01PM +0200, Paul B Mahol wrote: >>> >> Signed-off-by: Paul B Mahol <onemda@gmail.com> >>> >> --- >>> >> libavcodec/mjpegenc.c | 5 ++++- >>> >> libavcodec/mpegvideo_enc.c | 3 +-- >>> >> tests/lavf-regression.sh | 2 +- >>> >> 3 files changed, 6 insertions(+), 4 deletions(-) >>> > >>> > This doesnt work >>> >>> It does work, if you want full range set full range as option. >>> Check visually colors. >>> This happens because rgb is still marked incorrectly as limited range, >>> so limited is picked instead. >>> >>> >> Shouldn't this be part of the patchset? >> > > I thought that color_range only matters for YUV formats, for RGB it is > ignored. If it was decided to support limited RGB, then please update the > docs of AVCOL_RANGE_JPEG/MPEG. Otherwise the color_range of an RGB format > should not matter. > > On the other hand, if we decide to support limited RGB, then maybe a new > member in color_range is better, this way we are able to specify more > easily that we support limited YUV and full range RGB. (like ffplay > typically does). > Limited range RGB does NOT exist in the computer world, and I don't give a fuck if broadcasting uses it. I've fought hard to keep it from getting into AV1. So no, we will not be supporting limited range RGB.
On Wed, 2 May 2018 19:14:21 +0100 Rostislav Pehlivanov <atomnuker@gmail.com> wrote: > On 2 May 2018 at 19:04, Marton Balint <cus@passwd.hu> wrote: > > > > > > > On Tue, 1 May 2018, Rostislav Pehlivanov wrote: > > > > On 1 May 2018 at 23:22, Paul B Mahol <onemda@gmail.com> wrote: > >> > >> On 5/2/18, Michael Niedermayer <michael@niedermayer.cc> wrote: > >>> > Hi > >>> > > >>> > On Tue, May 01, 2018 at 09:40:01PM +0200, Paul B Mahol wrote: > >>> >> Signed-off-by: Paul B Mahol <onemda@gmail.com> > >>> >> --- > >>> >> libavcodec/mjpegenc.c | 5 ++++- > >>> >> libavcodec/mpegvideo_enc.c | 3 +-- > >>> >> tests/lavf-regression.sh | 2 +- > >>> >> 3 files changed, 6 insertions(+), 4 deletions(-) > >>> > > >>> > This doesnt work > >>> > >>> It does work, if you want full range set full range as option. > >>> Check visually colors. > >>> This happens because rgb is still marked incorrectly as limited range, > >>> so limited is picked instead. > >>> > >>> > >> Shouldn't this be part of the patchset? > >> > > > > I thought that color_range only matters for YUV formats, for RGB it is > > ignored. If it was decided to support limited RGB, then please update the > > docs of AVCOL_RANGE_JPEG/MPEG. Otherwise the color_range of an RGB format > > should not matter. > > > > On the other hand, if we decide to support limited RGB, then maybe a new > > member in color_range is better, this way we are able to specify more > > easily that we support limited YUV and full range RGB. (like ffplay > > typically does). > > > > Limited range RGB does NOT exist in the computer world, and I don't give a > fuck if broadcasting uses it. I've fought hard to keep it from getting into > AV1. > So no, we will not be supporting limited range RGB. You're very wrong. GPUs and HDMI (basically trash fire consumer garbage) sometimes seem to require you to feed limited RGB.
On 2 May 2018 at 19:16, wm4 <nfxjfg@googlemail.com> wrote: > On Wed, 2 May 2018 19:14:21 +0100 > Rostislav Pehlivanov <atomnuker@gmail.com> wrote: > > > On 2 May 2018 at 19:04, Marton Balint <cus@passwd.hu> wrote: > > > > > > > > > > > On Tue, 1 May 2018, Rostislav Pehlivanov wrote: > > > > > > On 1 May 2018 at 23:22, Paul B Mahol <onemda@gmail.com> wrote: > > >> > > >> On 5/2/18, Michael Niedermayer <michael@niedermayer.cc> wrote: > > >>> > Hi > > >>> > > > >>> > On Tue, May 01, 2018 at 09:40:01PM +0200, Paul B Mahol wrote: > > >>> >> Signed-off-by: Paul B Mahol <onemda@gmail.com> > > >>> >> --- > > >>> >> libavcodec/mjpegenc.c | 5 ++++- > > >>> >> libavcodec/mpegvideo_enc.c | 3 +-- > > >>> >> tests/lavf-regression.sh | 2 +- > > >>> >> 3 files changed, 6 insertions(+), 4 deletions(-) > > >>> > > > >>> > This doesnt work > > >>> > > >>> It does work, if you want full range set full range as option. > > >>> Check visually colors. > > >>> This happens because rgb is still marked incorrectly as limited > range, > > >>> so limited is picked instead. > > >>> > > >>> > > >> Shouldn't this be part of the patchset? > > >> > > > > > > I thought that color_range only matters for YUV formats, for RGB it is > > > ignored. If it was decided to support limited RGB, then please update > the > > > docs of AVCOL_RANGE_JPEG/MPEG. Otherwise the color_range of an RGB > format > > > should not matter. > > > > > > On the other hand, if we decide to support limited RGB, then maybe a > new > > > member in color_range is better, this way we are able to specify more > > > easily that we support limited YUV and full range RGB. (like ffplay > > > typically does). > > > > > > > Limited range RGB does NOT exist in the computer world, and I don't give > a > > fuck if broadcasting uses it. I've fought hard to keep it from getting > into > > AV1. > > So no, we will not be supporting limited range RGB. > > You're very wrong. GPUs and HDMI (basically trash fire consumer > garbage) sometimes seem to require you to feed limited RGB. > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > Maybe if you took a second to read what I wrote. HDMI sometimes requires limited range rgb because its derived from broadcasting junk where they had technical limitations they weren't smart enough to overcome. GPUs can generate or convert RGB to limited range only for transmission, through the use of gamma LUTs I believe. Either way, limited range RGB does NOT exist in computers, as in, not inside codecs, nor is there anything supporting such, and we will certainly not start doing it.
On Wed, 2 May 2018 19:37:27 +0100 Rostislav Pehlivanov <atomnuker@gmail.com> wrote: > On 2 May 2018 at 19:16, wm4 <nfxjfg@googlemail.com> wrote: > > > On Wed, 2 May 2018 19:14:21 +0100 > > Rostislav Pehlivanov <atomnuker@gmail.com> wrote: > > > > > On 2 May 2018 at 19:04, Marton Balint <cus@passwd.hu> wrote: > > > > > > > > > > > > > > > On Tue, 1 May 2018, Rostislav Pehlivanov wrote: > > > > > > > > On 1 May 2018 at 23:22, Paul B Mahol <onemda@gmail.com> wrote: > > > >> > > > >> On 5/2/18, Michael Niedermayer <michael@niedermayer.cc> wrote: > > > >>> > Hi > > > >>> > > > > >>> > On Tue, May 01, 2018 at 09:40:01PM +0200, Paul B Mahol wrote: > > > >>> >> Signed-off-by: Paul B Mahol <onemda@gmail.com> > > > >>> >> --- > > > >>> >> libavcodec/mjpegenc.c | 5 ++++- > > > >>> >> libavcodec/mpegvideo_enc.c | 3 +-- > > > >>> >> tests/lavf-regression.sh | 2 +- > > > >>> >> 3 files changed, 6 insertions(+), 4 deletions(-) > > > >>> > > > > >>> > This doesnt work > > > >>> > > > >>> It does work, if you want full range set full range as option. > > > >>> Check visually colors. > > > >>> This happens because rgb is still marked incorrectly as limited > > range, > > > >>> so limited is picked instead. > > > >>> > > > >>> > > > >> Shouldn't this be part of the patchset? > > > >> > > > > > > > > I thought that color_range only matters for YUV formats, for RGB it is > > > > ignored. If it was decided to support limited RGB, then please update > > the > > > > docs of AVCOL_RANGE_JPEG/MPEG. Otherwise the color_range of an RGB > > format > > > > should not matter. > > > > > > > > On the other hand, if we decide to support limited RGB, then maybe a > > new > > > > member in color_range is better, this way we are able to specify more > > > > easily that we support limited YUV and full range RGB. (like ffplay > > > > typically does). > > > > > > > > > > Limited range RGB does NOT exist in the computer world, and I don't give > > a > > > fuck if broadcasting uses it. I've fought hard to keep it from getting > > into > > > AV1. > > > So no, we will not be supporting limited range RGB. > > > > You're very wrong. GPUs and HDMI (basically trash fire consumer > > garbage) sometimes seem to require you to feed limited RGB. > > _______________________________________________ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > Maybe if you took a second to read what I wrote. HDMI sometimes requires > limited range rgb because its derived from broadcasting junk where they had > technical limitations they weren't smart enough to overcome. GPUs can > generate or convert RGB to limited range only for transmission, through the > use of gamma LUTs I believe. By that logic, aren't all mpeg style codecs (including vp9, av1, etc.) also "broadcasting junk"? I mean, even YUV could be full range if it weren't for broadcasting legacy. > Either way, limited range RGB does NOT exist in computers, as in, not > inside codecs, nor is there anything supporting such, and we will certainly > not start doing it. Denial of reality, I guess.
On 2 May 2018 at 19:46, wm4 <nfxjfg@googlemail.com> wrote: > On Wed, 2 May 2018 19:37:27 +0100 > Rostislav Pehlivanov <atomnuker@gmail.com> wrote: > > > On 2 May 2018 at 19:16, wm4 <nfxjfg@googlemail.com> wrote: > > > > > On Wed, 2 May 2018 19:14:21 +0100 > > > Rostislav Pehlivanov <atomnuker@gmail.com> wrote: > > > > > > > On 2 May 2018 at 19:04, Marton Balint <cus@passwd.hu> wrote: > > > > > > > > > > > > > > > > > > > On Tue, 1 May 2018, Rostislav Pehlivanov wrote: > > > > > > > > > > On 1 May 2018 at 23:22, Paul B Mahol <onemda@gmail.com> wrote: > > > > >> > > > > >> On 5/2/18, Michael Niedermayer <michael@niedermayer.cc> wrote: > > > > >>> > Hi > > > > >>> > > > > > >>> > On Tue, May 01, 2018 at 09:40:01PM +0200, Paul B Mahol wrote: > > > > >>> >> Signed-off-by: Paul B Mahol <onemda@gmail.com> > > > > >>> >> --- > > > > >>> >> libavcodec/mjpegenc.c | 5 ++++- > > > > >>> >> libavcodec/mpegvideo_enc.c | 3 +-- > > > > >>> >> tests/lavf-regression.sh | 2 +- > > > > >>> >> 3 files changed, 6 insertions(+), 4 deletions(-) > > > > >>> > > > > > >>> > This doesnt work > > > > >>> > > > > >>> It does work, if you want full range set full range as option. > > > > >>> Check visually colors. > > > > >>> This happens because rgb is still marked incorrectly as limited > > > range, > > > > >>> so limited is picked instead. > > > > >>> > > > > >>> > > > > >> Shouldn't this be part of the patchset? > > > > >> > > > > > > > > > > I thought that color_range only matters for YUV formats, for RGB > it is > > > > > ignored. If it was decided to support limited RGB, then please > update > > > the > > > > > docs of AVCOL_RANGE_JPEG/MPEG. Otherwise the color_range of an > RGB > > > format > > > > > should not matter. > > > > > > > > > > On the other hand, if we decide to support limited RGB, then maybe > a > > > new > > > > > member in color_range is better, this way we are able to specify > more > > > > > easily that we support limited YUV and full range RGB. (like ffplay > > > > > typically does). > > > > > > > > > > > > > Limited range RGB does NOT exist in the computer world, and I don't > give > > > a > > > > fuck if broadcasting uses it. I've fought hard to keep it from > getting > > > into > > > > AV1. > > > > So no, we will not be supporting limited range RGB. > > > > > > You're very wrong. GPUs and HDMI (basically trash fire consumer > > > garbage) sometimes seem to require you to feed limited RGB. > > > _______________________________________________ > > > ffmpeg-devel mailing list > > > ffmpeg-devel@ffmpeg.org > > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > Maybe if you took a second to read what I wrote. HDMI sometimes requires > > limited range rgb because its derived from broadcasting junk where they > had > > technical limitations they weren't smart enough to overcome. GPUs can > > generate or convert RGB to limited range only for transmission, through > the > > use of gamma LUTs I believe. > > By that logic, aren't all mpeg style codecs (including vp9, av1, etc.) > also "broadcasting junk"? I mean, even YUV could be full range if it > weren't for broadcasting legacy. > Not exactly, limited range YUV is a solution to an issue - out of gamut colors. Its not the way this should have been solved though, out of the few other ways I can think of. > Either way, limited range RGB does NOT exist in computers, as in, not > > inside codecs, nor is there anything supporting such, and we will > certainly > > not start doing it. > > Denial of reality, I guess. > Reality in a virtual world? No thanks, its whatever you make it.
diff --git a/libavcodec/mjpegenc.c b/libavcodec/mjpegenc.c index d2fcb8e191..c31985efd3 100644 --- a/libavcodec/mjpegenc.c +++ b/libavcodec/mjpegenc.c @@ -415,8 +415,10 @@ AVCodec ff_mjpeg_encoder = { .close = ff_mpv_encode_end, .capabilities = AV_CODEC_CAP_SLICE_THREADS | AV_CODEC_CAP_FRAME_THREADS | AV_CODEC_CAP_INTRA_ONLY, .pix_fmts = (const enum AVPixelFormat[]) { + AV_PIX_FMT_YUV420P, AV_PIX_FMT_YUV422P, AV_PIX_FMT_YUV444P, AV_PIX_FMT_YUVJ420P, AV_PIX_FMT_YUVJ422P, AV_PIX_FMT_YUVJ444P, AV_PIX_FMT_NONE }, + .color_ranges = (const enum AVColorRange[]){AVCOL_RANGE_JPEG, AVCOL_RANGE_MPEG, AVCOL_RANGE_UNSPECIFIED}, .priv_class = &mjpeg_class, }; #endif @@ -439,8 +441,9 @@ AVCodec ff_amv_encoder = { .encode2 = amv_encode_picture, .close = ff_mpv_encode_end, .pix_fmts = (const enum AVPixelFormat[]) { - AV_PIX_FMT_YUVJ420P, AV_PIX_FMT_NONE + AV_PIX_FMT_YUV420P, AV_PIX_FMT_YUVJ420P, AV_PIX_FMT_NONE }, + .color_ranges = (const enum AVColorRange[]){AVCOL_RANGE_JPEG, AVCOL_RANGE_UNSPECIFIED}, .priv_class = &amv_class, }; #endif diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c index 2e7950c64d..76aa675685 100644 --- a/libavcodec/mpegvideo_enc.c +++ b/libavcodec/mpegvideo_enc.c @@ -315,8 +315,7 @@ av_cold int ff_mpv_encode_init(AVCodecContext *avctx) avctx->pix_fmt == AV_PIX_FMT_YUV444P))) format_supported = 1; /* MPEG color space */ - else if (avctx->strict_std_compliance <= FF_COMPLIANCE_UNOFFICIAL && - (avctx->pix_fmt == AV_PIX_FMT_YUV420P || + else if ((avctx->pix_fmt == AV_PIX_FMT_YUV420P || avctx->pix_fmt == AV_PIX_FMT_YUV422P || avctx->pix_fmt == AV_PIX_FMT_YUV444P)) format_supported = 1; diff --git a/tests/lavf-regression.sh b/tests/lavf-regression.sh index 45c877e4ac..b03d5ac2e7 100755 --- a/tests/lavf-regression.sh +++ b/tests/lavf-regression.sh @@ -376,7 +376,7 @@ do_audio_only rso fi if [ -n "$do_smjpeg" ] ; then -do_lavf smjpeg "" "-f smjpeg" +do_lavf smjpeg "" "-vf format=yuv420p:pc -f smjpeg" fi if [ -n "$do_sox" ] ; then
Signed-off-by: Paul B Mahol <onemda@gmail.com> --- libavcodec/mjpegenc.c | 5 ++++- libavcodec/mpegvideo_enc.c | 3 +-- tests/lavf-regression.sh | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-)