[FFmpeg-devel] [PATCH] avutil: add av_get_colorspace_name()

[Michael Niedermayer]

On Sun, Sep 01, 2013 at 12:18:16AM +0200, Clément Bœsch wrote:
> Note: maybe we should try to consistent with the commit message prefix
> (lavu/lavf/lavfi vs avutil/avformat/avfilter)
> 
> On Sat, Aug 31, 2013 at 05:18:01PM +0200, Michael Niedermayer wrote:
> > Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
> > ---
> >  libavcodec/utils.c |   21 ++++++---------------
> >  libavutil/frame.c  |   18 ++++++++++++++++++
> >  libavutil/frame.h  |    2 ++
> >  3 files changed, 26 insertions(+), 15 deletions(-)
> > 
> > diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> > index faa6a16..4dc8748 100644
> > --- a/libavcodec/utils.c
> > +++ b/libavcodec/utils.c
> > @@ -2592,6 +2592,7 @@ void avcodec_string(char *buf, int buf_size, AVCodecContext *enc, int encode)
> >      case AVMEDIA_TYPE_VIDEO:
> >          if (enc->pix_fmt != AV_PIX_FMT_NONE) {
> >              char detail[256] = "(";
> > +            const char *colorspace_name;
> >              snprintf(buf + strlen(buf), buf_size - strlen(buf),
> >                       ", %s",
> >                       av_get_pix_fmt_name(enc->pix_fmt));
> > @@ -2601,21 +2602,11 @@ void avcodec_string(char *buf, int buf_size, AVCodecContext *enc, int encode)
> >              if (enc->color_range != AVCOL_RANGE_UNSPECIFIED)
> >                  av_strlcatf(detail, sizeof(detail),
> >                              enc->color_range == AVCOL_RANGE_MPEG ? "TV, ": "PC, ");
> > -            if (enc->colorspace<9U) {
> > -                static const char *name[] =  {
> > -                    "GBR",
> > -                    "bt709",
> > -                    NULL,
> > -                    NULL,
> > -                    "fcc",
> > -                    "bt470bg",
> > -                    "smpte170m",
> > -                    "smpte240m",
> > -                    "YCgCo",
> > -                };
> > -                if (name[enc->colorspace])
> > -                    av_strlcatf(detail, sizeof(detail), "%s, ", name[enc->colorspace]);
> > -            }
> > +
> > +            colorspace_name = av_get_colorspace_name(enc->colorspace);
> > +            if (colorspace_name)
> > +                av_strlcatf(detail, sizeof(detail), "%s, ", colorspace_name);
> > +
> 
> Do you plan to use it elsewhere?

no, but i guess GUI based tools, maybe ffprobe, ... ? could use it


> 
> >              if (strlen(detail) > 1) {
> >                  detail[strlen(detail) - 2] = 0;
> >                  av_strlcatf(buf, buf_size, "%s)", detail);
> > diff --git a/libavutil/frame.c b/libavutil/frame.c
> > index b0fdd49..4f2a5b8 100644
> > --- a/libavutil/frame.c
> > +++ b/libavutil/frame.c
> > @@ -70,6 +70,24 @@ int8_t *av_frame_get_qp_table(AVFrame *f, int *stride, int *type)
> >      return f->qp_table_buf->data;
> >  }
> >  
> > +const char *av_get_colorspace_name(enum AVColorSpace val)
> > +{
> > +    static const char *name[] =  {
> 
> nit/style: remove one extra space

removed


> 
> > +        "GBR",
> > +        "bt709",
> > +        NULL,
> > +        NULL,
> > +        "fcc",
> > +        "bt470bg",
> > +        "smpte170m",
> > +        "smpte240m",
> > +        "YCgCo",
> > +    };
> 

> Using designated initializers will remove the need of padding with NULL
> and the risk of error.

changed


> 
> Also, capitalization is inconsistent (and yes I know the issue is already
> present).
> 
> Q: why is AVCOL_SPC_RGB actually "GBR"?

the names are consistent with x264


> 
> > +    if (val < 0 || val >= FF_ARRAY_ELEMS(name))
> > +        return NULL;
> > +    return name[val];
> > +}
> > +
> >  static void get_frame_defaults(AVFrame *frame)
> >  {
> >      if (frame->extended_data != frame->data)
> > diff --git a/libavutil/frame.h b/libavutil/frame.h
> > index 3313703..98410b9 100644
> > --- a/libavutil/frame.h
> > +++ b/libavutil/frame.h
> > @@ -508,6 +508,8 @@ void    av_frame_set_colorspace(AVFrame *frame, enum AVColorSpace val);
> >  enum AVColorRange av_frame_get_color_range(const AVFrame *frame);
> >  void    av_frame_set_color_range(AVFrame *frame, enum AVColorRange val);
> >  
> > +const char *av_get_colorspace_name(enum AVColorSpace val);
> > +
> 
> Here is a free doxycookie slightly customized from avcodec_get_name():
> 
> /**
>  * Get the name of a colorspace.
>  * @return a static string identifying the codec; can be NULL.
>  */

added


> 
> [...]
> 
> Missing minor bump, otherwise should be OK.

will bump when i comit it


> 
> -- 
> Clément B.



> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If a bugfix only changes things apparently unrelated to the bug with no
further explanation, that is a good sign that the bugfix is wrong.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130901/9c2382bf/attachment.asc>