[FFmpeg-devel] libavformat/yuv4mpeg: Add color range support for Y4M Add color_range support in Y4M. Also set pixel format and color_range for YUVJ pixel formats.

Submitted by Wang Cao on June 13, 2018, 4:03 p.m.

Details

Message ID 20180613160313.164743-1-wangcao@google.com
State New
Headers show

Commit Message

Wang Cao June 13, 2018, 4:03 p.m.
---
 libavformat/yuv4mpegdec.c |  8 ++++++++
 libavformat/yuv4mpegenc.c | 37 +++++++++++++++++++++++++++++++++++--
 2 files changed, 43 insertions(+), 2 deletions(-)

Comments

Carl Eugen Hoyos June 13, 2018, 4:18 p.m.
2018-06-13 18:03 GMT+02:00, Wang Cao <doubleecao@gmail.com>:

> @@ -220,6 +221,12 @@ static int yuv4_read_header(AVFormatContext *s)
>                      alt_pix_fmt = AV_PIX_FMT_YUV422P;
>                  else if (strncmp("444", tokstart, 3) == 0)
>                      alt_pix_fmt = AV_PIX_FMT_YUV444P;
> +            } else if (strncmp("COLORRANGE=", tokstart, 11) == 0) {

Where is this specified?

Carl Eugen
Wang Cao June 20, 2018, 9:42 a.m.
It's in the yuv4mpegenc.c. I added a support for color range by specifying
metadata in the Y4M header.
On Thu, Jun 14, 2018 at 12:19 AM Carl Eugen Hoyos <ceffmpeg@gmail.com>
wrote:

> 2018-06-13 18:03 GMT+02:00, Wang Cao <doubleecao@gmail.com>:
>
> > @@ -220,6 +221,12 @@ static int yuv4_read_header(AVFormatContext *s)
> >                      alt_pix_fmt = AV_PIX_FMT_YUV422P;
> >                  else if (strncmp("444", tokstart, 3) == 0)
> >                      alt_pix_fmt = AV_PIX_FMT_YUV444P;
> > +            } else if (strncmp("COLORRANGE=", tokstart, 11) == 0) {
>
> Where is this specified?

Carl Eugen
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
Carl Eugen Hoyos June 21, 2018, 9:32 a.m.
2018-06-20 11:42 GMT+02:00, Wang Cao <doubleecao@gmail.com>:
> It's in the yuv4mpegenc.c. I added a support for color range
> by specifying metadata in the Y4M header.

Yes, I understand.

My question was which authority defined this metadata for
y4m? Or which (non-FFmpeg-based) other software
understands the metadata your patch adds to the files.

Please do not top-post here, Carl Eugen
Wang Cao June 22, 2018, 7:29 a.m.
>
> My question was which authority defined this metadata for
> y4m? Or which (non-FFmpeg-based) other software
> understands the metadata your patch adds to the files.
>
AFAIK, no other software and authority has defined this metadata
for color range. According to https://linux.die.net/man/5/yuv4mpeg,
we should be able to use X tag to support color range.

Color range would be good to have to give Y4M more capability to
handle HDR(probably) and other pixel formats (.e.g. YUV420P with
full range).
Carl Eugen Hoyos June 22, 2018, 1:25 p.m.
2018-06-22 9:29 GMT+02:00, Wang Cao <doubleecao@gmail.com>:
>>
>> My question was which authority defined this metadata for
>> y4m? Or which (non-FFmpeg-based) other software
>> understands the metadata your patch adds to the files.
>>
> AFAIK, no other software and authority has defined this metadata
> for color range. According to https://linux.die.net/man/5/yuv4mpeg,
> we should be able to use X tag to support color range.

No more comments from me.

Carl Eugen
Sasi Inguva June 25, 2018, 8:25 p.m.
Friendly ping! Thx.

On Fri, Jun 22, 2018 at 6:25 AM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:

> 2018-06-22 9:29 GMT+02:00, Wang Cao <doubleecao@gmail.com>:
> >>
> >> My question was which authority defined this metadata for
> >> y4m? Or which (non-FFmpeg-based) other software
> >> understands the metadata your patch adds to the files.
> >>
> > AFAIK, no other software and authority has defined this metadata
> > for color range. According to https://linux.die.net/man/5/yuv4mpeg,
> > we should be able to use X tag to support color range.
>
> No more comments from me.
>
> Carl Eugen
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
Michael Niedermayer June 25, 2018, 11:37 p.m.
On Thu, Jun 14, 2018 at 12:03:13AM +0800, Wang Cao wrote:
> ---
>  libavformat/yuv4mpegdec.c |  8 ++++++++
>  libavformat/yuv4mpegenc.c | 37 +++++++++++++++++++++++++++++++++++--
>  2 files changed, 43 insertions(+), 2 deletions(-)
> 
> diff --git a/libavformat/yuv4mpegdec.c b/libavformat/yuv4mpegdec.c
> index eff7fc518e..86e8673b2f 100644
> --- a/libavformat/yuv4mpegdec.c
> +++ b/libavformat/yuv4mpegdec.c
> @@ -41,6 +41,7 @@ static int yuv4_read_header(AVFormatContext *s)
>      enum AVPixelFormat pix_fmt = AV_PIX_FMT_NONE, alt_pix_fmt = AV_PIX_FMT_NONE;
>      enum AVChromaLocation chroma_sample_location = AVCHROMA_LOC_UNSPECIFIED;
>      enum AVFieldOrder field_order = AV_FIELD_UNKNOWN;
> +    enum AVColorRange color_range = AVCOL_RANGE_UNSPECIFIED;
>      AVStream *st;
>  
>      for (i = 0; i < MAX_YUV4_HEADER; i++) {
> @@ -220,6 +221,12 @@ static int yuv4_read_header(AVFormatContext *s)
>                      alt_pix_fmt = AV_PIX_FMT_YUV422P;
>                  else if (strncmp("444", tokstart, 3) == 0)
>                      alt_pix_fmt = AV_PIX_FMT_YUV444P;
> +            } else if (strncmp("COLORRANGE=", tokstart, 11) == 0) {
> +              tokstart += 11;
> +              if (strncmp("JPEG",tokstart, 4) == 0)
> +                  color_range = AVCOL_RANGE_JPEG;
> +              else if (strncmp("MPEG", tokstart, 4) == 0)
> +                  color_range = AVCOL_RANGE_MPEG;
>              }

If this is a type we are choosing then it would be probably better
to use something else than mpeg/jpeg as names
these are 2 standard comittees and their standards support more than
one color range.

maybe "full" and "limited" or some other terms may be better

thanks


[...]
Wang Cao June 28, 2018, 8:28 a.m.
>
> If this is a type we are choosing then it would be probably better
> to use something else than mpeg/jpeg as names
> these are 2 standard comittees and their standards support more than
> one color range.
> maybe "full" and "limited" or some other terms may be better

Thanks for pointing out this. I feel "MPEG" and "JPEG" is more consistent
with the defined variable names (.e.g AVCOL_RANGE_MPEG and
AVCOL_RANGE_JPEG). If you think "FULL" and "LIMITED" look better to you, it
also looks good to me not to use "MPEG" and "JPEG" names.
Wang Cao June 28, 2018, 8:32 a.m.
I created a newer version to change "MPEG" to "LIMITED" and "JPEG" to
"FULL".
Michael Niedermayer June 28, 2018, 11:54 p.m.
On Thu, Jun 28, 2018 at 04:28:24PM +0800, Wang Cao wrote:
> >
> > If this is a type we are choosing then it would be probably better
> > to use something else than mpeg/jpeg as names
> > these are 2 standard comittees and their standards support more than
> > one color range.
> > maybe "full" and "limited" or some other terms may be better
> 
> Thanks for pointing out this. I feel "MPEG" and "JPEG" is more consistent
> with the defined variable names (.e.g AVCOL_RANGE_MPEG and
> AVCOL_RANGE_JPEG). If you think "FULL" and "LIMITED" look better to you, it
> also looks good to me not to use "MPEG" and "JPEG" names.

We should not "extend" fileformats with (bad) internal name choices from
our source code


[...]

Patch hide | download patch | download mbox

diff --git a/libavformat/yuv4mpegdec.c b/libavformat/yuv4mpegdec.c
index eff7fc518e..86e8673b2f 100644
--- a/libavformat/yuv4mpegdec.c
+++ b/libavformat/yuv4mpegdec.c
@@ -41,6 +41,7 @@  static int yuv4_read_header(AVFormatContext *s)
     enum AVPixelFormat pix_fmt = AV_PIX_FMT_NONE, alt_pix_fmt = AV_PIX_FMT_NONE;
     enum AVChromaLocation chroma_sample_location = AVCHROMA_LOC_UNSPECIFIED;
     enum AVFieldOrder field_order = AV_FIELD_UNKNOWN;
+    enum AVColorRange color_range = AVCOL_RANGE_UNSPECIFIED;
     AVStream *st;
 
     for (i = 0; i < MAX_YUV4_HEADER; i++) {
@@ -220,6 +221,12 @@  static int yuv4_read_header(AVFormatContext *s)
                     alt_pix_fmt = AV_PIX_FMT_YUV422P;
                 else if (strncmp("444", tokstart, 3) == 0)
                     alt_pix_fmt = AV_PIX_FMT_YUV444P;
+            } else if (strncmp("COLORRANGE=", tokstart, 11) == 0) {
+              tokstart += 11;
+              if (strncmp("JPEG",tokstart, 4) == 0)
+                  color_range = AVCOL_RANGE_JPEG;
+              else if (strncmp("MPEG", tokstart, 4) == 0)
+                  color_range = AVCOL_RANGE_MPEG;
             }
             while (tokstart < header_end && *tokstart != 0x20)
                 tokstart++;
@@ -263,6 +270,7 @@  static int yuv4_read_header(AVFormatContext *s)
     st->codecpar->codec_id            = AV_CODEC_ID_RAWVIDEO;
     st->sample_aspect_ratio           = (AVRational){ aspectn, aspectd };
     st->codecpar->chroma_location     = chroma_sample_location;
+    st->codecpar->color_range         = color_range;
     st->codecpar->field_order         = field_order;
     s->packet_size = av_image_get_buffer_size(st->codecpar->format, width, height, 1) + Y4M_FRAME_MAGIC_LEN;
     if ((int) s->packet_size < 0)
diff --git a/libavformat/yuv4mpegenc.c b/libavformat/yuv4mpegenc.c
index 44f40bbad9..555ef15105 100644
--- a/libavformat/yuv4mpegenc.c
+++ b/libavformat/yuv4mpegenc.c
@@ -33,6 +33,7 @@  static int yuv4_generate_header(AVFormatContext *s, char* buf)
     int raten, rated, aspectn, aspectd, n;
     char inter;
     const char *colorspace = "";
+    const char *colorrange = "";
     int field_order;
 
     st     = s->streams[0];
@@ -57,6 +58,17 @@  static int yuv4_generate_header(AVFormatContext *s, char* buf)
     FF_ENABLE_DEPRECATION_WARNINGS
 #endif
 
+    switch(st->codecpar->color_range) {
+    case AVCOL_RANGE_MPEG:
+        colorrange = " XCOLORRANGE=MPEG";
+        break;
+    case AVCOL_RANGE_JPEG:
+        colorrange = " XCOLORRANGE=JPEG";
+        break;
+    default:
+        break;
+    }
+
     switch (field_order) {
     case AV_FIELD_TB:
     case AV_FIELD_TT: inter = 't'; break;
@@ -84,6 +96,18 @@  static int yuv4_generate_header(AVFormatContext *s, char* buf)
     case AV_PIX_FMT_YUV411P:
         colorspace = " C411 XYSCSS=411";
         break;
+    case AV_PIX_FMT_YUVJ420P:
+        colorspace = " C420jpeg XYSCSS=420JPEG";
+        colorrange = " XCOLORRANGE=JPEG";
+        break;
+    case AV_PIX_FMT_YUVJ422P:
+        colorspace = " C422 XYSCSS=422";
+        colorrange = " XCOLORRANGE=JPEG";
+        break;
+    case AV_PIX_FMT_YUVJ444P:
+        colorspace = " C444 XYSCSS=444";
+        colorrange = " XCOLORRANGE=JPEG";
+        break;
     case AV_PIX_FMT_YUV420P:
         switch (st->codecpar->chroma_location) {
         case AVCHROMA_LOC_TOPLEFT: colorspace = " C420paldv XYSCSS=420PALDV"; break;
@@ -145,13 +169,14 @@  static int yuv4_generate_header(AVFormatContext *s, char* buf)
     }
 
     /* construct stream header, if this is the first frame */
-    n = snprintf(buf, Y4M_LINE_MAX, "%s W%d H%d F%d:%d I%c A%d:%d%s\n",
+    n = snprintf(buf, Y4M_LINE_MAX, "%s W%d H%d F%d:%d I%c A%d:%d%s%s\n",
                  Y4M_MAGIC, width, height, raten, rated, inter,
-                 aspectn, aspectd, colorspace);
+                 aspectn, aspectd, colorspace, colorrange);
 
     return n;
 }
 
+
 static int yuv4_write_packet(AVFormatContext *s, AVPacket *pkt)
 {
     AVStream *st = s->streams[pkt->stream_index];
@@ -192,6 +217,10 @@  static int yuv4_write_packet(AVFormatContext *s, AVPacket *pkt)
     case AV_PIX_FMT_YUV420P:
     case AV_PIX_FMT_YUV422P:
     case AV_PIX_FMT_YUV444P:
+    // TODO: remove YUVJ pixel formats when they are completely removed from the codebase.
+    case AV_PIX_FMT_YUVJ420P:
+    case AV_PIX_FMT_YUVJ422P:
+    case AV_PIX_FMT_YUVJ444P:
         break;
     case AV_PIX_FMT_GRAY9:
     case AV_PIX_FMT_GRAY10:
@@ -271,6 +300,10 @@  static int yuv4_write_header(AVFormatContext *s)
     case AV_PIX_FMT_YUV420P:
     case AV_PIX_FMT_YUV422P:
     case AV_PIX_FMT_YUV444P:
+    // TODO: remove YUVJ pixel formats when they are completely removed from the codebase.
+    case AV_PIX_FMT_YUVJ420P:
+    case AV_PIX_FMT_YUVJ422P:
+    case AV_PIX_FMT_YUVJ444P:
         break;
     case AV_PIX_FMT_GRAY9:
     case AV_PIX_FMT_GRAY10: