diff mbox

[FFmpeg-devel] hevc: Add support for alternative transfer characterics SEI

Message ID 20170609213531.58481-1-vittorio.giovara@gmail.com
State Superseded
Headers show

Commit Message

Vittorio Giovara June 9, 2017, 9:35 p.m. UTC 
Signed-off-by: Vittorio Giovara <vittorio.giovara@gmail.com>
---
 libavcodec/hevc_sei.c | 9 +++++++++
 libavcodec/hevc_sei.h | 7 +++++++
 libavcodec/hevcdec.c  | 5 +++++
 3 files changed, 21 insertions(+)

Comments

James Almer June 11, 2017, 2:19 p.m. UTC | #1 
On 6/9/2017 6:35 PM, Vittorio Giovara wrote:
> Signed-off-by: Vittorio Giovara <vittorio.giovara@gmail.com>
> ---
>  libavcodec/hevc_sei.c | 9 +++++++++
>  libavcodec/hevc_sei.h | 7 +++++++
>  libavcodec/hevcdec.c  | 5 +++++
>  3 files changed, 21 insertions(+)
> 
> diff --git a/libavcodec/hevc_sei.c b/libavcodec/hevc_sei.c
> index f0ad84f2f4..cd55d50212 100644
> --- a/libavcodec/hevc_sei.c
> +++ b/libavcodec/hevc_sei.c
> @@ -265,6 +265,13 @@ static int decode_nal_sei_active_parameter_sets(HEVCSEIContext *s, GetBitContext
>      return 0;
>  }
>  
> +static int decode_nal_sei_alternative_transfer(HEVCSEIAlternativeTransfer *s, GetBitContext *gb)
> +{
> +    s->present = 1;
> +    s->preferred_transfer_characteristics = get_bits(gb, 8);
> +    return 0;
> +}
> +
>  static int decode_nal_sei_prefix(GetBitContext *gb, HEVCSEIContext *s, const HEVCParamSets *ps,
>                                   int type, int size, void *logctx)
>  {
> @@ -285,6 +292,8 @@ static int decode_nal_sei_prefix(GetBitContext *gb, HEVCSEIContext *s, const HEV
>          return decode_nal_sei_active_parameter_sets(s, gb, logctx);
>      case HEVC_SEI_TYPE_USER_DATA_REGISTERED_ITU_T_T35:
>          return decode_nal_sei_user_data_registered_itu_t_t35(s, gb, size);
> +    case HEVC_SEI_TYPE_ALTERNATIVE_TRANSFER_CHARACTERISTICS:
> +        return decode_nal_sei_alternative_transfer(&s->alternative_transfer, gb);
>      default:
>          av_log(logctx, AV_LOG_DEBUG, "Skipped PREFIX SEI %d\n", type);
>          skip_bits_long(gb, 8 * size);
> diff --git a/libavcodec/hevc_sei.h b/libavcodec/hevc_sei.h
> index 5645e4f5de..1b522227ec 100644
> --- a/libavcodec/hevc_sei.h
> +++ b/libavcodec/hevc_sei.h
> @@ -56,6 +56,7 @@ typedef enum {
>      HEVC_SEI_TYPE_REGION_REFRESH_INFO                  = 134,
>      HEVC_SEI_TYPE_MASTERING_DISPLAY_INFO               = 137,
>      HEVC_SEI_TYPE_CONTENT_LIGHT_LEVEL_INFO             = 144,
> +    HEVC_SEI_TYPE_ALTERNATIVE_TRANSFER_CHARACTERISTICS = 147,
>  } HEVC_SEI_Type;
>  
>  typedef struct HEVCSEIPictureHash {
> @@ -100,6 +101,11 @@ typedef struct HEVCSEIContentLight {
>      uint16_t max_pic_average_light_level;
>  } HEVCSEIContentLight;
>  
> +typedef struct HEVCSEIAlternativeTransfer {
> +    int present;
> +    int preferred_transfer_characteristics;
> +} HEVCSEIAlternativeTransfer;
> +
>  typedef struct HEVCSEIContext {
>      HEVCSEIPictureHash picture_hash;
>      HEVCSEIFramePacking frame_packing;
> @@ -109,6 +115,7 @@ typedef struct HEVCSEIContext {
>      HEVCSEIMasteringDisplay mastering_display;
>      HEVCSEIContentLight content_light;
>      int active_seq_parameter_set_id;
> +    HEVCSEIAlternativeTransfer alternative_transfer;
>  } HEVCSEIContext;
>  
>  struct HEVCParamSets;
> diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
> index 23a89345a8..955cc703cf 100644
> --- a/libavcodec/hevcdec.c
> +++ b/libavcodec/hevcdec.c
> @@ -2684,6 +2684,11 @@ static int set_side_data(HEVCContext *s)
>          s->avctx->properties |= FF_CODEC_PROPERTY_CLOSED_CAPTIONS;
>      }
>  
> +    if (s->sei.alternative_transfer.present &&
> +        s->sei.alternative_transfer.preferred_transfer_characteristics != AVCOL_SPC_UNSPECIFIED) {

Shouldn't this be AVCOL_TRC_UNSPECIFIED?
And validate it with av_color_transfer_name() instead, like it's done in
hevc_ps.c with the VUI value.

> +        s->avctx->color_trc = s->sei.alternative_transfer.preferred_transfer_characteristics;

Did you make sure avctx->color_trc will not be overwritten by the VUI
value after this? As in, what's called first, set_side_data() or
export_stream_params()? And is it properly propagated in updates between
thread contexts?

> +    }
> +
>      return 0;
>  }
>  
>
Vittorio Giovara June 13, 2017, 4:48 p.m. UTC | #2 
On Sun, Jun 11, 2017 at 10:19 AM, James Almer <jamrial@gmail.com> wrote:
> On 6/9/2017 6:35 PM, Vittorio Giovara wrote:
>> Signed-off-by: Vittorio Giovara <vittorio.giovara@gmail.com>
>> ---
>>  libavcodec/hevc_sei.c | 9 +++++++++
>>  libavcodec/hevc_sei.h | 7 +++++++
>>  libavcodec/hevcdec.c  | 5 +++++
>>  3 files changed, 21 insertions(+)
>>
>> diff --git a/libavcodec/hevc_sei.c b/libavcodec/hevc_sei.c
>> index f0ad84f2f4..cd55d50212 100644
>> --- a/libavcodec/hevc_sei.c
>> +++ b/libavcodec/hevc_sei.c
>> @@ -265,6 +265,13 @@ static int decode_nal_sei_active_parameter_sets(HEVCSEIContext *s, GetBitContext
>>      return 0;
>>  }
>>
>> +static int decode_nal_sei_alternative_transfer(HEVCSEIAlternativeTransfer *s, GetBitContext *gb)
>> +{
>> +    s->present = 1;
>> +    s->preferred_transfer_characteristics = get_bits(gb, 8);
>> +    return 0;
>> +}
>> +
>>  static int decode_nal_sei_prefix(GetBitContext *gb, HEVCSEIContext *s, const HEVCParamSets *ps,
>>                                   int type, int size, void *logctx)
>>  {
>> @@ -285,6 +292,8 @@ static int decode_nal_sei_prefix(GetBitContext *gb, HEVCSEIContext *s, const HEV
>>          return decode_nal_sei_active_parameter_sets(s, gb, logctx);
>>      case HEVC_SEI_TYPE_USER_DATA_REGISTERED_ITU_T_T35:
>>          return decode_nal_sei_user_data_registered_itu_t_t35(s, gb, size);
>> +    case HEVC_SEI_TYPE_ALTERNATIVE_TRANSFER_CHARACTERISTICS:
>> +        return decode_nal_sei_alternative_transfer(&s->alternative_transfer, gb);
>>      default:
>>          av_log(logctx, AV_LOG_DEBUG, "Skipped PREFIX SEI %d\n", type);
>>          skip_bits_long(gb, 8 * size);
>> diff --git a/libavcodec/hevc_sei.h b/libavcodec/hevc_sei.h
>> index 5645e4f5de..1b522227ec 100644
>> --- a/libavcodec/hevc_sei.h
>> +++ b/libavcodec/hevc_sei.h
>> @@ -56,6 +56,7 @@ typedef enum {
>>      HEVC_SEI_TYPE_REGION_REFRESH_INFO                  = 134,
>>      HEVC_SEI_TYPE_MASTERING_DISPLAY_INFO               = 137,
>>      HEVC_SEI_TYPE_CONTENT_LIGHT_LEVEL_INFO             = 144,
>> +    HEVC_SEI_TYPE_ALTERNATIVE_TRANSFER_CHARACTERISTICS = 147,
>>  } HEVC_SEI_Type;
>>
>>  typedef struct HEVCSEIPictureHash {
>> @@ -100,6 +101,11 @@ typedef struct HEVCSEIContentLight {
>>      uint16_t max_pic_average_light_level;
>>  } HEVCSEIContentLight;
>>
>> +typedef struct HEVCSEIAlternativeTransfer {
>> +    int present;
>> +    int preferred_transfer_characteristics;
>> +} HEVCSEIAlternativeTransfer;
>> +
>>  typedef struct HEVCSEIContext {
>>      HEVCSEIPictureHash picture_hash;
>>      HEVCSEIFramePacking frame_packing;
>> @@ -109,6 +115,7 @@ typedef struct HEVCSEIContext {
>>      HEVCSEIMasteringDisplay mastering_display;
>>      HEVCSEIContentLight content_light;
>>      int active_seq_parameter_set_id;
>> +    HEVCSEIAlternativeTransfer alternative_transfer;
>>  } HEVCSEIContext;
>>
>>  struct HEVCParamSets;
>> diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
>> index 23a89345a8..955cc703cf 100644
>> --- a/libavcodec/hevcdec.c
>> +++ b/libavcodec/hevcdec.c
>> @@ -2684,6 +2684,11 @@ static int set_side_data(HEVCContext *s)
>>          s->avctx->properties |= FF_CODEC_PROPERTY_CLOSED_CAPTIONS;
>>      }
>>
>> +    if (s->sei.alternative_transfer.present &&
>> +        s->sei.alternative_transfer.preferred_transfer_characteristics != AVCOL_SPC_UNSPECIFIED) {
>
> Shouldn't this be AVCOL_TRC_UNSPECIFIED?

Right, typo, thanks.

> And validate it with av_color_transfer_name() instead, like it's done in
> hevc_ps.c with the VUI value.
>
>> +        s->avctx->color_trc = s->sei.alternative_transfer.preferred_transfer_characteristics;
>
> Did you make sure avctx->color_trc will not be overwritten by the VUI
> value after this? As in, what's called first, set_side_data() or
> export_stream_params()? And is it properly propagated in updates between
> thread contexts?

This is called after extradata decoding: from hevc_decode_frame(),
first hevc_decode_extradata() -> export_stream_param(), then
decode_nal_units() -> decode_nal_unit() -> hevc_frame_start() ->
set_side_data().
Regarding your second question, I suppose so, the sei check just above
modifies the global context in a similar way.

One thing I noticed is that I probably need to update the output frame
value besides the context, I'll send an updated version.
--
Vittorio
diff mbox

Patch

diff --git a/libavcodec/hevc_sei.c b/libavcodec/hevc_sei.c
index f0ad84f2f4..cd55d50212 100644
--- a/libavcodec/hevc_sei.c
+++ b/libavcodec/hevc_sei.c
@@ -265,6 +265,13 @@  static int decode_nal_sei_active_parameter_sets(HEVCSEIContext *s, GetBitContext
     return 0;
 }
 
+static int decode_nal_sei_alternative_transfer(HEVCSEIAlternativeTransfer *s, GetBitContext *gb)
+{
+    s->present = 1;
+    s->preferred_transfer_characteristics = get_bits(gb, 8);
+    return 0;
+}
+
 static int decode_nal_sei_prefix(GetBitContext *gb, HEVCSEIContext *s, const HEVCParamSets *ps,
                                  int type, int size, void *logctx)
 {
@@ -285,6 +292,8 @@  static int decode_nal_sei_prefix(GetBitContext *gb, HEVCSEIContext *s, const HEV
         return decode_nal_sei_active_parameter_sets(s, gb, logctx);
     case HEVC_SEI_TYPE_USER_DATA_REGISTERED_ITU_T_T35:
         return decode_nal_sei_user_data_registered_itu_t_t35(s, gb, size);
+    case HEVC_SEI_TYPE_ALTERNATIVE_TRANSFER_CHARACTERISTICS:
+        return decode_nal_sei_alternative_transfer(&s->alternative_transfer, gb);
     default:
         av_log(logctx, AV_LOG_DEBUG, "Skipped PREFIX SEI %d\n", type);
         skip_bits_long(gb, 8 * size);
diff --git a/libavcodec/hevc_sei.h b/libavcodec/hevc_sei.h
index 5645e4f5de..1b522227ec 100644
--- a/libavcodec/hevc_sei.h
+++ b/libavcodec/hevc_sei.h
@@ -56,6 +56,7 @@  typedef enum {
     HEVC_SEI_TYPE_REGION_REFRESH_INFO                  = 134,
     HEVC_SEI_TYPE_MASTERING_DISPLAY_INFO               = 137,
     HEVC_SEI_TYPE_CONTENT_LIGHT_LEVEL_INFO             = 144,
+    HEVC_SEI_TYPE_ALTERNATIVE_TRANSFER_CHARACTERISTICS = 147,
 } HEVC_SEI_Type;
 
 typedef struct HEVCSEIPictureHash {
@@ -100,6 +101,11 @@  typedef struct HEVCSEIContentLight {
     uint16_t max_pic_average_light_level;
 } HEVCSEIContentLight;
 
+typedef struct HEVCSEIAlternativeTransfer {
+    int present;
+    int preferred_transfer_characteristics;
+} HEVCSEIAlternativeTransfer;
+
 typedef struct HEVCSEIContext {
     HEVCSEIPictureHash picture_hash;
     HEVCSEIFramePacking frame_packing;
@@ -109,6 +115,7 @@  typedef struct HEVCSEIContext {
     HEVCSEIMasteringDisplay mastering_display;
     HEVCSEIContentLight content_light;
     int active_seq_parameter_set_id;
+    HEVCSEIAlternativeTransfer alternative_transfer;
 } HEVCSEIContext;
 
 struct HEVCParamSets;
diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
index 23a89345a8..955cc703cf 100644
--- a/libavcodec/hevcdec.c
+++ b/libavcodec/hevcdec.c
@@ -2684,6 +2684,11 @@  static int set_side_data(HEVCContext *s)
         s->avctx->properties |= FF_CODEC_PROPERTY_CLOSED_CAPTIONS;
     }
 
+    if (s->sei.alternative_transfer.present &&
+        s->sei.alternative_transfer.preferred_transfer_characteristics != AVCOL_SPC_UNSPECIFIED) {
+        s->avctx->color_trc = s->sei.alternative_transfer.preferred_transfer_characteristics;
+    }
+
     return 0;
 }