diff mbox

[FFmpeg-devel,v2] avfilter/showinfo: support Content Light Level information

Message ID 20190810145830.12284-1-lance.lmwang@gmail.com
State New
Headers show

Commit Message

Lance Wang Aug. 10, 2019, 2:58 p.m. UTC 
From: Limin Wang <lance.lmwang@gmail.com>

show real information instead of the unknown side data type message for HDR10 stream

Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
---
 libavfilter/vf_showinfo.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

Comments

Lance Wang Aug. 17, 2019, 2:43 p.m. UTC | #1 
ping, please help to merge the code.

On Sat, Aug 10, 2019 at 10:58:30PM +0800, lance.lmwang@gmail.com wrote:
> From: Limin Wang <lance.lmwang@gmail.com>
> 
> show real information instead of the unknown side data type message for HDR10 stream
> 
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
>  libavfilter/vf_showinfo.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/libavfilter/vf_showinfo.c b/libavfilter/vf_showinfo.c
> index 9fa322e..94d17bf 100644
> --- a/libavfilter/vf_showinfo.c
> +++ b/libavfilter/vf_showinfo.c
> @@ -160,6 +160,20 @@ static void dump_mastering_display(AVFilterContext *ctx, AVFrameSideData *sd)
>             av_q2d(mastering_display->min_luminance), av_q2d(mastering_display->max_luminance));
>  }
>  
> +static void dump_content_light_metadata(AVFilterContext *ctx, AVFrameSideData *sd)
> +{
> +    AVContentLightMetadata* metadata = (AVContentLightMetadata*)sd->data;
> +
> +    if (sd->size < sizeof(*metadata)) {
> +        av_log(ctx, AV_LOG_ERROR, "invalid data");
> +        return;
> +    }
> +
> +    av_log(ctx, AV_LOG_INFO, "Content Light Level information: "
> +           "MaxCLL=%d, MaxFALL=%d",
> +           metadata->MaxCLL, metadata->MaxFALL);
> +}
> +
>  static void dump_color_property(AVFilterContext *ctx, AVFrame *frame)
>  {
>      const char *color_range_str     = av_color_range_name(frame->color_range);
> @@ -301,6 +315,9 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *frame)
>          case AV_FRAME_DATA_MASTERING_DISPLAY_METADATA:
>              dump_mastering_display(ctx, sd);
>              break;
> +        case AV_FRAME_DATA_CONTENT_LIGHT_LEVEL:
> +            dump_content_light_metadata(ctx, sd);
> +            break;
>          default:
>              av_log(ctx, AV_LOG_WARNING, "unknown side data type %d (%d bytes)",
>                     sd->type, sd->size);
> -- 
> 2.6.4
>
Lance Wang Aug. 17, 2019, 2:58 p.m. UTC | #2 
ping, please help to merge the code if it's OK.

On Sat, Aug 10, 2019 at 10:58:30PM +0800, lance.lmwang@gmail.com wrote:
> From: Limin Wang <lance.lmwang@gmail.com>
> 
> show real information instead of the unknown side data type message for HDR10 stream
> 
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
>  libavfilter/vf_showinfo.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/libavfilter/vf_showinfo.c b/libavfilter/vf_showinfo.c
> index 9fa322e..94d17bf 100644
> --- a/libavfilter/vf_showinfo.c
> +++ b/libavfilter/vf_showinfo.c
> @@ -160,6 +160,20 @@ static void dump_mastering_display(AVFilterContext *ctx, AVFrameSideData *sd)
>             av_q2d(mastering_display->min_luminance), av_q2d(mastering_display->max_luminance));
>  }
>  
> +static void dump_content_light_metadata(AVFilterContext *ctx, AVFrameSideData *sd)
> +{
> +    AVContentLightMetadata* metadata = (AVContentLightMetadata*)sd->data;
> +
> +    if (sd->size < sizeof(*metadata)) {
> +        av_log(ctx, AV_LOG_ERROR, "invalid data");
> +        return;
> +    }
> +
> +    av_log(ctx, AV_LOG_INFO, "Content Light Level information: "
> +           "MaxCLL=%d, MaxFALL=%d",
> +           metadata->MaxCLL, metadata->MaxFALL);
> +}
> +
>  static void dump_color_property(AVFilterContext *ctx, AVFrame *frame)
>  {
>      const char *color_range_str     = av_color_range_name(frame->color_range);
> @@ -301,6 +315,9 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *frame)
>          case AV_FRAME_DATA_MASTERING_DISPLAY_METADATA:
>              dump_mastering_display(ctx, sd);
>              break;
> +        case AV_FRAME_DATA_CONTENT_LIGHT_LEVEL:
> +            dump_content_light_metadata(ctx, sd);
> +            break;
>          default:
>              av_log(ctx, AV_LOG_WARNING, "unknown side data type %d (%d bytes)",
>                     sd->type, sd->size);
> -- 
> 2.6.4
>
James Almer Aug. 17, 2019, 5:44 p.m. UTC | #3 
On 8/10/2019 11:58 AM, lance.lmwang@gmail.com wrote:
> From: Limin Wang <lance.lmwang@gmail.com>
> 
> show real information instead of the unknown side data type message for HDR10 stream
> 
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
>  libavfilter/vf_showinfo.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/libavfilter/vf_showinfo.c b/libavfilter/vf_showinfo.c
> index 9fa322e..94d17bf 100644
> --- a/libavfilter/vf_showinfo.c
> +++ b/libavfilter/vf_showinfo.c
> @@ -160,6 +160,20 @@ static void dump_mastering_display(AVFilterContext *ctx, AVFrameSideData *sd)
>             av_q2d(mastering_display->min_luminance), av_q2d(mastering_display->max_luminance));
>  }
>  
> +static void dump_content_light_metadata(AVFilterContext *ctx, AVFrameSideData *sd)
> +{
> +    AVContentLightMetadata* metadata = (AVContentLightMetadata*)sd->data;
> +
> +    if (sd->size < sizeof(*metadata)) {

sizeof(AVContentLightMetadata) is not part of the ABI, so just remove
this check.

> +        av_log(ctx, AV_LOG_ERROR, "invalid data");
> +        return;
> +    }
> +
> +    av_log(ctx, AV_LOG_INFO, "Content Light Level information: "
> +           "MaxCLL=%d, MaxFALL=%d",
> +           metadata->MaxCLL, metadata->MaxFALL);
> +}
> +
>  static void dump_color_property(AVFilterContext *ctx, AVFrame *frame)
>  {
>      const char *color_range_str     = av_color_range_name(frame->color_range);
> @@ -301,6 +315,9 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *frame)
>          case AV_FRAME_DATA_MASTERING_DISPLAY_METADATA:
>              dump_mastering_display(ctx, sd);
>              break;
> +        case AV_FRAME_DATA_CONTENT_LIGHT_LEVEL:
> +            dump_content_light_metadata(ctx, sd);
> +            break;
>          default:
>              av_log(ctx, AV_LOG_WARNING, "unknown side data type %d (%d bytes)",
>                     sd->type, sd->size);
>
James Almer Aug. 17, 2019, 5:49 p.m. UTC | #4 
On 8/17/2019 2:44 PM, James Almer wrote:
> On 8/10/2019 11:58 AM, lance.lmwang@gmail.com wrote:
>> From: Limin Wang <lance.lmwang@gmail.com>
>>
>> show real information instead of the unknown side data type message for HDR10 stream
>>
>> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
>> ---
>>  libavfilter/vf_showinfo.c | 17 +++++++++++++++++
>>  1 file changed, 17 insertions(+)
>>
>> diff --git a/libavfilter/vf_showinfo.c b/libavfilter/vf_showinfo.c
>> index 9fa322e..94d17bf 100644
>> --- a/libavfilter/vf_showinfo.c
>> +++ b/libavfilter/vf_showinfo.c
>> @@ -160,6 +160,20 @@ static void dump_mastering_display(AVFilterContext *ctx, AVFrameSideData *sd)
>>             av_q2d(mastering_display->min_luminance), av_q2d(mastering_display->max_luminance));
>>  }
>>  
>> +static void dump_content_light_metadata(AVFilterContext *ctx, AVFrameSideData *sd)
>> +{
>> +    AVContentLightMetadata* metadata = (AVContentLightMetadata*)sd->data;
>> +
>> +    if (sd->size < sizeof(*metadata)) {
> 
> sizeof(AVContentLightMetadata) is not part of the ABI, so just remove
> this check.

Oh, your first version was like that, and this one is the result of
someone suggesting to add the check.

Pushed the correct version then.

> 
>> +        av_log(ctx, AV_LOG_ERROR, "invalid data");
>> +        return;
>> +    }
>> +
>> +    av_log(ctx, AV_LOG_INFO, "Content Light Level information: "
>> +           "MaxCLL=%d, MaxFALL=%d",
>> +           metadata->MaxCLL, metadata->MaxFALL);
>> +}
>> +
>>  static void dump_color_property(AVFilterContext *ctx, AVFrame *frame)
>>  {
>>      const char *color_range_str     = av_color_range_name(frame->color_range);
>> @@ -301,6 +315,9 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *frame)
>>          case AV_FRAME_DATA_MASTERING_DISPLAY_METADATA:
>>              dump_mastering_display(ctx, sd);
>>              break;
>> +        case AV_FRAME_DATA_CONTENT_LIGHT_LEVEL:
>> +            dump_content_light_metadata(ctx, sd);
>> +            break;
>>          default:
>>              av_log(ctx, AV_LOG_WARNING, "unknown side data type %d (%d bytes)",
>>                     sd->type, sd->size);
>>
>
Lance Wang Aug. 17, 2019, 11:09 p.m. UTC | #5 
On Sat, Aug 17, 2019 at 02:49:29PM -0300, James Almer wrote:
> On 8/17/2019 2:44 PM, James Almer wrote:
> > On 8/10/2019 11:58 AM, lance.lmwang@gmail.com wrote:
> >> From: Limin Wang <lance.lmwang@gmail.com>
> >>
> >> show real information instead of the unknown side data type message for HDR10 stream
> >>
> >> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> >> ---
> >>  libavfilter/vf_showinfo.c | 17 +++++++++++++++++
> >>  1 file changed, 17 insertions(+)
> >>
> >> diff --git a/libavfilter/vf_showinfo.c b/libavfilter/vf_showinfo.c
> >> index 9fa322e..94d17bf 100644
> >> --- a/libavfilter/vf_showinfo.c
> >> +++ b/libavfilter/vf_showinfo.c
> >> @@ -160,6 +160,20 @@ static void dump_mastering_display(AVFilterContext *ctx, AVFrameSideData *sd)
> >>             av_q2d(mastering_display->min_luminance), av_q2d(mastering_display->max_luminance));
> >>  }
> >>  
> >> +static void dump_content_light_metadata(AVFilterContext *ctx, AVFrameSideData *sd)
> >> +{
> >> +    AVContentLightMetadata* metadata = (AVContentLightMetadata*)sd->data;
> >> +
> >> +    if (sd->size < sizeof(*metadata)) {
> > 
> > sizeof(AVContentLightMetadata) is not part of the ABI, so just remove
> > this check.
> 
> Oh, your first version was like that, and this one is the result of
> someone suggesting to add the check.
> 
> Pushed the correct version then.

thanks, when we should check the size as some other metadata have the
size check?

> 
> > 
> >> +        av_log(ctx, AV_LOG_ERROR, "invalid data");
> >> +        return;
> >> +    }
> >> +
> >> +    av_log(ctx, AV_LOG_INFO, "Content Light Level information: "
> >> +           "MaxCLL=%d, MaxFALL=%d",
> >> +           metadata->MaxCLL, metadata->MaxFALL);
> >> +}
> >> +
> >>  static void dump_color_property(AVFilterContext *ctx, AVFrame *frame)
> >>  {
> >>      const char *color_range_str     = av_color_range_name(frame->color_range);
> >> @@ -301,6 +315,9 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *frame)
> >>          case AV_FRAME_DATA_MASTERING_DISPLAY_METADATA:
> >>              dump_mastering_display(ctx, sd);
> >>              break;
> >> +        case AV_FRAME_DATA_CONTENT_LIGHT_LEVEL:
> >> +            dump_content_light_metadata(ctx, sd);
> >> +            break;
> >>          default:
> >>              av_log(ctx, AV_LOG_WARNING, "unknown side data type %d (%d bytes)",
> >>                     sd->type, sd->size);
> >>
> > 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
James Almer Aug. 17, 2019, 11:14 p.m. UTC | #6 
On 8/17/2019 8:09 PM, Limin Wang wrote:
> On Sat, Aug 17, 2019 at 02:49:29PM -0300, James Almer wrote:
>> On 8/17/2019 2:44 PM, James Almer wrote:
>>> On 8/10/2019 11:58 AM, lance.lmwang@gmail.com wrote:
>>>> From: Limin Wang <lance.lmwang@gmail.com>
>>>>
>>>> show real information instead of the unknown side data type message for HDR10 stream
>>>>
>>>> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
>>>> ---
>>>>  libavfilter/vf_showinfo.c | 17 +++++++++++++++++
>>>>  1 file changed, 17 insertions(+)
>>>>
>>>> diff --git a/libavfilter/vf_showinfo.c b/libavfilter/vf_showinfo.c
>>>> index 9fa322e..94d17bf 100644
>>>> --- a/libavfilter/vf_showinfo.c
>>>> +++ b/libavfilter/vf_showinfo.c
>>>> @@ -160,6 +160,20 @@ static void dump_mastering_display(AVFilterContext *ctx, AVFrameSideData *sd)
>>>>             av_q2d(mastering_display->min_luminance), av_q2d(mastering_display->max_luminance));
>>>>  }
>>>>  
>>>> +static void dump_content_light_metadata(AVFilterContext *ctx, AVFrameSideData *sd)
>>>> +{
>>>> +    AVContentLightMetadata* metadata = (AVContentLightMetadata*)sd->data;
>>>> +
>>>> +    if (sd->size < sizeof(*metadata)) {
>>>
>>> sizeof(AVContentLightMetadata) is not part of the ABI, so just remove
>>> this check.
>>
>> Oh, your first version was like that, and this one is the result of
>> someone suggesting to add the check.
>>
>> Pushed the correct version then.
> 
> thanks, when we should check the size as some other metadata have the
> size check?

When the doxy for the relevant struct or the side data type states its
size is part of the ABI.
Some are (AVReplayGain, display matrix, etc), and some aren't
(AVDynamicHDRPlus, AVContentLightMetadata, AVMasteringDisplayMetadata,
AVSphericalMapping, etc).
> 
>>
>>>
>>>> +        av_log(ctx, AV_LOG_ERROR, "invalid data");
>>>> +        return;
>>>> +    }
>>>> +
>>>> +    av_log(ctx, AV_LOG_INFO, "Content Light Level information: "
>>>> +           "MaxCLL=%d, MaxFALL=%d",
>>>> +           metadata->MaxCLL, metadata->MaxFALL);
>>>> +}
>>>> +
>>>>  static void dump_color_property(AVFilterContext *ctx, AVFrame *frame)
>>>>  {
>>>>      const char *color_range_str     = av_color_range_name(frame->color_range);
>>>> @@ -301,6 +315,9 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *frame)
>>>>          case AV_FRAME_DATA_MASTERING_DISPLAY_METADATA:
>>>>              dump_mastering_display(ctx, sd);
>>>>              break;
>>>> +        case AV_FRAME_DATA_CONTENT_LIGHT_LEVEL:
>>>> +            dump_content_light_metadata(ctx, sd);
>>>> +            break;
>>>>          default:
>>>>              av_log(ctx, AV_LOG_WARNING, "unknown side data type %d (%d bytes)",
>>>>                     sd->type, sd->size);
>>>>
>>>
>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
diff mbox

Patch

diff --git a/libavfilter/vf_showinfo.c b/libavfilter/vf_showinfo.c
index 9fa322e..94d17bf 100644
--- a/libavfilter/vf_showinfo.c
+++ b/libavfilter/vf_showinfo.c
@@ -160,6 +160,20 @@  static void dump_mastering_display(AVFilterContext *ctx, AVFrameSideData *sd)
            av_q2d(mastering_display->min_luminance), av_q2d(mastering_display->max_luminance));
 }
 
+static void dump_content_light_metadata(AVFilterContext *ctx, AVFrameSideData *sd)
+{
+    AVContentLightMetadata* metadata = (AVContentLightMetadata*)sd->data;
+
+    if (sd->size < sizeof(*metadata)) {
+        av_log(ctx, AV_LOG_ERROR, "invalid data");
+        return;
+    }
+
+    av_log(ctx, AV_LOG_INFO, "Content Light Level information: "
+           "MaxCLL=%d, MaxFALL=%d",
+           metadata->MaxCLL, metadata->MaxFALL);
+}
+
 static void dump_color_property(AVFilterContext *ctx, AVFrame *frame)
 {
     const char *color_range_str     = av_color_range_name(frame->color_range);
@@ -301,6 +315,9 @@  static int filter_frame(AVFilterLink *inlink, AVFrame *frame)
         case AV_FRAME_DATA_MASTERING_DISPLAY_METADATA:
             dump_mastering_display(ctx, sd);
             break;
+        case AV_FRAME_DATA_CONTENT_LIGHT_LEVEL:
+            dump_content_light_metadata(ctx, sd);
+            break;
         default:
             av_log(ctx, AV_LOG_WARNING, "unknown side data type %d (%d bytes)",
                    sd->type, sd->size);