[FFmpeg-devel] [PATCH v2] cbs_h265: Add a lot more SEI parsing support

[Mark Thompson]

On 11/11/18 17:32, James Almer wrote:
> On 11/11/2018 2:27 PM, Mark Thompson wrote:
>> Supports both prefix and suffix SEI, decoding all of the common SEI
>> types and some more obscure ones.  Most of this is tested by the
>> existing tests in fate.
>> ---
>> On 11/11/18 15:57, James Almer wrote:
>>> On 10/27/2018 6:39 PM, Mark Thompson wrote:
>>>> Supports both prefix and suffix SEI, decoding all of the common SEI
>>>> types and some more obscure ones.  Most of this is tested by the
>>>> existing tests in fate.
>>>
>>> It would be neat to make the *_metadata filters not decompose unit types
>>> it doesn't care about (based on user options). Before this patch, all
>>> these SEI messages were simply copied as is, but after it they will be
>>> fully parsed even in pass through mode, potentially slowing down the
>>> process.
>>>
>>> Of course, doing that would make the cbs tests useless. Maybe the
>>> default behavior could be to not decompose what's not going to be
>>> modified, and an option to enable it could be added that the test would use.
>>>
>>>> ---
>>>>  libavcodec/cbs_h2645.c                |  20 +-
>>>>  libavcodec/cbs_h265.h                 | 124 +++++++
>>>>  libavcodec/cbs_h265_syntax_template.c | 497 +++++++++++++++++++++++++-
>>>>  libavcodec/hevc.h                     |   3 +
>>>>  libavcodec/hevc_sei.h                 |   1 +
>>>>  5 files changed, 625 insertions(+), 20 deletions(-)
>>>>
>>>
>>>> diff --git a/libavcodec/hevc_sei.h b/libavcodec/hevc_sei.h
>>>> index e92da25bbf..2fec00ace0 100644
>>>> --- a/libavcodec/hevc_sei.h
>>>> +++ b/libavcodec/hevc_sei.h
>>>> @@ -52,6 +52,7 @@ typedef enum {
>>>>      HEVC_SEI_TYPE_DECODED_PICTURE_HASH                 = 132,
>>>>      HEVC_SEI_TYPE_SCALABLE_NESTING                     = 133,
>>>>      HEVC_SEI_TYPE_REGION_REFRESH_INFO                  = 134,
>>>> +    HEVC_SEI_TYPE_TIME_CODE                            = 136,
>>>>      HEVC_SEI_TYPE_MASTERING_DISPLAY_INFO               = 137,
>>>>      HEVC_SEI_TYPE_CONTENT_LIGHT_LEVEL_INFO             = 144,
>>>>      HEVC_SEI_TYPE_ALTERNATIVE_TRANSFER_CHARACTERISTICS = 147,
>>>
>>> Adding this one should be trivial, btw. It's a single field, and there
>>> are real world samples out there. Support for it was added to the
>>> decoder some time ago.
>>
>> Sure, here's a v2 with that added.
>>
>> Thanks,
>>
>> - Mark
>>
>>
>>  libavcodec/cbs_h2645.c                |  20 +-
>>  libavcodec/cbs_h265.h                 | 130 +++++++
>>  libavcodec/cbs_h265_syntax_template.c | 512 +++++++++++++++++++++++++-
>>  libavcodec/hevc.h                     |   3 +
>>  libavcodec/hevc_sei.h                 |   1 +
>>  5 files changed, 646 insertions(+), 20 deletions(-)
>>
>> diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c
>> index e55bd00183..86baf129ce 100644
>> --- a/libavcodec/cbs_h2645.c
>> +++ b/libavcodec/cbs_h2645.c
>> @@ -490,9 +490,21 @@ static void cbs_h265_free_slice(void *unit, uint8_t *content)
>>  static void cbs_h265_free_sei_payload(H265RawSEIPayload *payload)
>>  {
>>      switch (payload->payload_type) {
>> +    case HEVC_SEI_TYPE_BUFFERING_PERIOD:
>> +    case HEVC_SEI_TYPE_PICTURE_TIMING:
>> +    case HEVC_SEI_TYPE_PAN_SCAN_RECT:
>> +    case HEVC_SEI_TYPE_RECOVERY_POINT:
>> +    case HEVC_SEI_TYPE_DISPLAY_ORIENTATION:
>> +    case HEVC_SEI_TYPE_DECODED_PICTURE_HASH:
>>      case HEVC_SEI_TYPE_MASTERING_DISPLAY_INFO:
>>      case HEVC_SEI_TYPE_CONTENT_LIGHT_LEVEL_INFO:
> 
> Shouldn't atc be listed here?

Yes!  And so should active-parameter-sets and time-code.

Added locally.

Thanks,

- Mark

>>          break;
>> +    case HEVC_SEI_TYPE_USER_DATA_REGISTERED_ITU_T_T35:
>> +        av_buffer_unref(&payload->payload.user_data_registered.data_ref);
>> +        break;
>> +    case HEVC_SEI_TYPE_USER_DATA_UNREGISTERED:
>> +        av_buffer_unref(&payload->payload.user_data_unregistered.data_ref);
>> +        break;
>>      default:
>>          av_buffer_unref(&payload->payload.other.data_ref);
>>          break;