[FFmpeg-devel] [PATCH v11 3/4] avfilter/vf_showinfo: display H.26[45] user data unregistered sei message

[lance.lmwang at gmail.com]

On Wed, Jun 10, 2020 at 08:13:59PM +0200, Andreas Rheinhardt wrote:
> lance.lmwang at gmail.com:
> > From: Limin Wang <lance.lmwang at gmail.com>
> > 
> > Signed-off-by: Limin Wang <lance.lmwang at gmail.com>
> > ---
> >  libavfilter/vf_showinfo.c | 37 +++++++++++++++++++++++++++++++++++++
> >  1 file changed, 37 insertions(+)
> > 
> > diff --git a/libavfilter/vf_showinfo.c b/libavfilter/vf_showinfo.c
> > index 5d4aee4..2511da5 100644
> > --- a/libavfilter/vf_showinfo.c
> > +++ b/libavfilter/vf_showinfo.c
> > @@ -37,6 +37,7 @@
> >  #include "libavutil/timecode.h"
> >  #include "libavutil/mastering_display_metadata.h"
> >  #include "libavutil/video_enc_params.h"
> > +#include "libavutil/avstring.h"
> >  
> >  #include "avfilter.h"
> >  #include "internal.h"
> > @@ -190,6 +191,39 @@ static void dump_video_enc_params(AVFilterContext *ctx, AVFrameSideData *sd)
> >          av_log(ctx, AV_LOG_INFO, "%u blocks; ", par->nb_blocks);
> >  }
> >  
> > +static int string_is_print(const uint8_t *str)
> > +{
> > +    while (av_isgraph(*str)) str++;
> > +    return !*str;
> 
> This is dangerous: The SEI message needn't be zero-terminated, so this
> may segfault. Furthermore, it is also wrong: If the user data payload
> happens to contain a binary zero preceded by printable ASCII characters,
> it will be considered a string, even if there are unprintable characters
> after the binary zero (which may really happen if the payload is
> actually binary).

I'll consider to alloc one zero padding byte memory for user data to force
zero-terminated. For the second question, we'll print the printable string,
after the binary zero will ignored, I think it's expected? or do you want to
dump the left binary by hex? If so it's considerable to dump them by hex,
although it's not friendly if the data is string. 

> 
> > +}
> > +
> > +static void dump_sei_unregistered_metadata(AVFilterContext *ctx, AVFrameSideData *sd)
> > +{
> > +    const int uuid_size = 16;
> > +    uint8_t *user_data = sd->data;
> > +
> > +    if (sd->size < uuid_size) {
> > +        av_log(ctx, AV_LOG_ERROR, "invalid data(%d < UUID(%d-bytes))", sd->size, uuid_size);
> > +        return;
> > +    }
> > +
> > +    av_log(ctx, AV_LOG_INFO, "User Data Unregistered:\n");
> > +    av_log(ctx, AV_LOG_INFO, "UUID=");
> > +    for (int i = 0; i < uuid_size; i++) {
> > +        av_log(ctx, AV_LOG_INFO, "%02x", user_data[i]);
> > +        if (i == 3 || i == 5 || i == 7 || i == 9)
> > +            av_log(ctx, AV_LOG_INFO, "-");
> > +    }
> > +    av_log(ctx, AV_LOG_INFO, "\n");
> > +
> > +    user_data += uuid_size;
> > +    /* Only print the user data details if it's string */
> > +    if (string_is_print(user_data)) {
> > +        av_log(ctx, AV_LOG_INFO, "User Data=");
> > +        av_log(ctx, AV_LOG_INFO, "%s", user_data);
> 
> Given that the user_data needn't be zero-terminated, you must specify
> the precision (i.e. the number of bytes to write) here.
> 
> > +    }
> > +}
> > +
> >  static void dump_color_property(AVFilterContext *ctx, AVFrame *frame)
> >  {
> >      const char *color_range_str     = av_color_range_name(frame->color_range);
> > @@ -375,6 +409,9 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *frame)
> >          case AV_FRAME_DATA_VIDEO_ENC_PARAMS:
> >              dump_video_enc_params(ctx, sd);
> >              break;
> > +        case AV_FRAME_DATA_SEI_UNREGISTERED:
> > +            dump_sei_unregistered_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 at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".

-- 
Thanks,
Limin Wang