[FFmpeg-devel] [PATCH] Add frame side data when SEI green metadata are detected

Nicolas Derouineau nicolas.derouineau at vitec.com
Tue Jan 5 11:19:28 CET 2016


>this should be in a seperate patch and libavutil/version.h should have
>its minor version bumped

Ok, so I'm going to do two separate patch (commit ?) for libavformat and for libavutil.
Where should I bump libavutil/version.h ? (I'm not sure I really understand this action).

>GreenMetaData is defined in h264.h, which is an internal header
>while AVFrameSideDataType is external. public API should not refer to
>internal/private headers

Ok

>also the struct, as currently defined is platform specific. Theres no
>gurantee that a compiler doesnt add padding betwen the elements. 

I'm not sure how to deal with this issue in ffmpeg. I guess #pragma packed is not an option, right ?

>Also the code initializing it only matches the platform dependant struct
>on little endian

Can you explain to me which part of the code is platform dependant on little endian ? (because memset seems allright to me, but I guess I'm wrong).

Thanks,

Nicolas DEROUINEAU
Research Engineer
VITEC

T.  +33 1 46 73 06 06
E.  nicolas.derouineau at vitec.com
W. www.vitec.com

________________________________________
De : ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> de la part de Michael Niedermayer <michael at niedermayer.cc>
Envoyé : lundi 4 janvier 2016 19:37
À : FFmpeg development discussions and patches
Objet : Re: [FFmpeg-devel] [PATCH] Add frame side data when SEI green   metadata are detected

On Mon, Jan 04, 2016 at 02:50:44PM +0100, Nicolas DEROUINEAU wrote:
> ---
>  libavcodec/h264.c     | 21 +++++++++++++++++++++
>  libavcodec/h264.h     |  1 +
>  libavcodec/h264_sei.c |  3 +++

>  libavutil/frame.h     |  8 ++++++++

this should be in a seperate patch and libavutil/version.h should have
its minor version bumped


>  4 files changed, 33 insertions(+)
>
> diff --git a/libavcodec/h264.c b/libavcodec/h264.c
> index 089a86f..e90bcc0 100644
> --- a/libavcodec/h264.c
> +++ b/libavcodec/h264.c
> @@ -879,6 +879,27 @@ static void decode_postinit(H264Context *h, int setup_finished)
>          }
>      }
>
> +    if (h->sei_green_metadata_present) {
> +        AVFrameSideData *Greenmd = av_frame_new_side_data(cur->f, AV_FRAME_DATA_GREENMD,
> +                                                    sizeof(GreenMetaData));
> +        if (Greenmd) {
> +            memset((uint8_t*)Greenmd->data, 0, sizeof(GreenMetaData));
> +            Greenmd->data[0] = h->sei_green_metadata.green_metadata_type;
> +            Greenmd->data[1] = h->sei_green_metadata.period_type;
> +            Greenmd->data[2] = (uint8_t)(h->sei_green_metadata.num_seconds>>8);
> +            Greenmd->data[3] = (uint8_t)(h->sei_green_metadata.num_seconds&0xFF);
> +            Greenmd->data[4] = (uint8_t)(h->sei_green_metadata.num_pictures>>8);
> +            Greenmd->data[5] = (uint8_t)(h->sei_green_metadata.num_pictures&0xFF);
> +            Greenmd->data[6] = h->sei_green_metadata.percent_non_zero_macroblocks;
> +            Greenmd->data[7] = h->sei_green_metadata.percent_intra_coded_macroblocks;
> +            Greenmd->data[8] = h->sei_green_metadata.percent_six_tap_filtering;
> +            Greenmd->data[9] = h->sei_green_metadata.percent_alpha_point_deblocking_instance;
> +            Greenmd->data[10] = h->sei_green_metadata.xsd_metric_type;
> +            Greenmd->data[11] = (uint8_t)(h->sei_green_metadata.xsd_metric_value>>8);
> +            Greenmd->data[12] = (uint8_t)(h->sei_green_metadata.xsd_metric_value&0xFF);
> +        }
> +    }
> +
>      if (h->sei_reguserdata_afd_present) {
>          AVFrameSideData *sd = av_frame_new_side_data(cur->f, AV_FRAME_DATA_AFD,
>                                                       sizeof(uint8_t));
> diff --git a/libavcodec/h264.h b/libavcodec/h264.h
> index 5d9aecd..51490d6 100644
> --- a/libavcodec/h264.h
> +++ b/libavcodec/h264.h
> @@ -839,6 +839,7 @@ typedef struct H264Context {
>      qpel_mc_func (*qpel_avg)[16];
>
>      /*Green Metadata */
> +    int sei_green_metadata_present;
>      GreenMetaData sei_green_metadata;
>
>  } H264Context;
> diff --git a/libavcodec/h264_sei.c b/libavcodec/h264_sei.c
> index 0411b87..4a021b8 100644
> --- a/libavcodec/h264_sei.c
> +++ b/libavcodec/h264_sei.c
> @@ -43,6 +43,7 @@ void ff_h264_reset_sei(H264Context *h)
>      h->sei_frame_packing_present    =  0;
>      h->sei_display_orientation_present = 0;
>      h->sei_reguserdata_afd_present  =  0;
> +    h->sei_green_metadata_present   =  0;
>
>      h->a53_caption_size = 0;
>      av_freep(&h->a53_caption);
> @@ -363,6 +364,8 @@ static int decode_GreenMetadata(H264Context *h)
>      if (h->avctx->debug & FF_DEBUG_GREEN_MD)
>          av_log(h->avctx, AV_LOG_DEBUG,          "Green Metadata Info SEI message\n");
>
> +    h->sei_green_metadata_present = 1;
> +
>      h->sei_green_metadata.green_metadata_type=get_bits(&h->gb, 8);
>
>      if (h->avctx->debug & FF_DEBUG_GREEN_MD)


> diff --git a/libavutil/frame.h b/libavutil/frame.h
> index 9c6061a..89a57ad 100644
> --- a/libavutil/frame.h
> +++ b/libavutil/frame.h
> @@ -112,6 +112,14 @@ enum AVFrameSideDataType {
>       * enum AVAudioServiceType defined in avcodec.h.
>       */
>      AV_FRAME_DATA_AUDIO_SERVICE_TYPE,
> +
> +
> +    /**
> +     * This side data must be associated with a video frame and corresponds to
> +     * struct GreenMDataType defined in avcodec.h. The green metadata specification
> +     * is given in ISO/IEC 23001.

GreenMDataType does not exist
GreenMetaData is defined in h264.h, which is an internal header
while AVFrameSideDataType is external. public API should not refer to
internal/private headers
also the struct, as currently defined is platform specific. Theres no
gurantee that a compiler doesnt add padding betwen the elements. Also
the code initializing it only matches the platform dependant struct
on little endian


[...]

--
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I am the wisest man alive, for I know one thing, and that is that I know
nothing. -- Socrates


More information about the ffmpeg-devel mailing list