[FFmpeg-devel] [PATCH] pgssubdec: fix subpicture output colorspace and range

[wm4]

On Sat, 23 Apr 2016 13:53:00 +0200
Hendrik Leppkes <h.leppkes at gmail.com> wrote:

> On Sat, Apr 23, 2016 at 1:18 PM, wm4 <nfxjfg at googlemail.com> wrote:
> > On Fri, 22 Apr 2016 23:06:37 +0300
> > Jan Ekström <jeebjp at gmail.com> wrote:
> >  
> >> Functionality used before didn't widen the values from limited to
> >> full range. Additionally, now the decoder uses BT.709 where it
> >> should be used according to the video resolution.
> >>
> >> Default for not yet set colorimetry is BT.709 due to most observed
> >> HDMV content being HD.
> >>
> >> BT.709 coefficients were gathered from the first two parts of BT.709
> >> to BT.2020 conversion guide in ARIB STD-B62 (Pt. 1, Chapter 6.2.2).
> >>
> >> Based on a patch by Carl Eugen Hoyos.
> >> ---
> >>  libavcodec/pgssubdec.c | 20 ++++++++++++++++++--
> >>  libavutil/colorspace.h | 10 ++++++++++
> >>  2 files changed, 28 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/libavcodec/pgssubdec.c b/libavcodec/pgssubdec.c
> >> index 07a2a78..4145f1c 100644
> >> --- a/libavcodec/pgssubdec.c
> >> +++ b/libavcodec/pgssubdec.c
> >> @@ -96,6 +96,7 @@ typedef struct PGSSubContext {
> >>      PGSSubPalettes     palettes;
> >>      PGSSubObjects      objects;
> >>      int forced_subs_only;
> >> +    int hdtv;
> >>  } PGSSubContext;
> >>
> >>  static void flush_cache(AVCodecContext *avctx)
> >> @@ -136,6 +137,9 @@ static PGSSubPalette * find_palette(int id, PGSSubPalettes *palettes)
> >>
> >>  static av_cold int init_decoder(AVCodecContext *avctx)
> >>  {
> >> +    PGSSubContext *ctx = avctx->priv_data;
> >> +    ctx->hdtv          = -1;
> >> +
> >>      avctx->pix_fmt     = AV_PIX_FMT_PAL8;
> >>
> >>      return 0;
> >> @@ -354,8 +358,14 @@ static int parse_palette_segment(AVCodecContext *avctx,
> >>          cb        = bytestream_get_byte(&buf);
> >>          alpha     = bytestream_get_byte(&buf);
> >>
> >> -        YUV_TO_RGB1(cb, cr);
> >> -        YUV_TO_RGB2(r, g, b, y);
> >> +        /* Default to HDTV (-1 or 1) */
> >> +        if (ctx->hdtv) {
> >> +            YUV_TO_RGB1_CCIR_BT709(cb, cr);
> >> +        } else {
> >> +            YUV_TO_RGB1_CCIR(cb, cr);
> >> +        }
> >> +
> >> +        YUV_TO_RGB2_CCIR(r, g, b, y);
> >>
> >>          ff_dlog(avctx, "Color %d := (%d,%d,%d,%d)\n", color_id, r, g, b, alpha);
> >>
> >> @@ -388,6 +398,12 @@ static int parse_presentation_segment(AVCodecContext *avctx,
> >>      int w = bytestream_get_be16(&buf);
> >>      int h = bytestream_get_be16(&buf);
> >>
> >> +    // Set colorimetry according to resolution
> >> +    if (h > 576)
> >> +        ctx->hdtv = 1;
> >> +    else
> >> +        ctx->hdtv = 0;
> >> +
> >>      ctx->presentation.pts = pts;
> >>
> >>      ff_dlog(avctx, "Video Dimensions %dx%d\n",
> >> diff --git a/libavutil/colorspace.h b/libavutil/colorspace.h
> >> index 826ffd5..7d3f711 100644
> >> --- a/libavutil/colorspace.h
> >> +++ b/libavutil/colorspace.h
> >> @@ -41,6 +41,16 @@
> >>      b_add = FIX(1.77200*255.0/224.0) * cb + ONE_HALF;\
> >>  }
> >>
> >> +#define YUV_TO_RGB1_CCIR_BT709(cb1, cr1)\
> >> +{\
> >> +    cb = (cb1) - 128;\
> >> +    cr = (cr1) - 128;\
> >> +    r_add = FIX(1.5747*255.0/224.0) * cr + ONE_HALF;\
> >> +    g_add = - FIX(0.1873*255.0/224.0) * cb - FIX(0.4682*255.0/224.0) * cr + \
> >> +            ONE_HALF;\
> >> +    b_add = FIX(1.8556*255.0/224.0) * cb + ONE_HALF;\
> >> +}
> >> +
> >>  #define YUV_TO_RGB2_CCIR(r, g, b, y1)\
> >>  {\
> >>      y = ((y1) - 16) * FIX(255.0/219.0);\  
> >
> > This still has the problem that the palette could be read before the
> > resolution is known.  
> 
> The Blu-ray specification controls the order of segments, and this
> should not happen on conformant streams.
> Otherwise, I think Carl's suggestion might help, PGS subtitles are
> generally from Blu-rays, which means most of it is HD, so swapping the
> flag to detect SD conditions might make it act more "appropriate" in
> absence of w/h.

In that case the hdtv field should be completely removed, and the test
put in the palette conversion code.