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

[Hendrik Leppkes]

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.

- Hendrik