[FFmpeg-devel] libavcodec/h264_sei: Don't log random user data.

From d43c0366d4ba128772dc7909152f4e9635f269cb Mon Sep 17 00:00:00 2001
From: Kieran Kunhya <kierank@obe.tv>
Date: Fri, 5 May 2017 14:29:59 -0700
Subject: [PATCH] libavcodec/h264_sei: Don't log random user data. This
 prevents terminal junk.

---
 libavcodec/h264_sei.c | 3 ---
 1 file changed, 3 deletions(-)

     return 0;
 }

On Fri, May 05, 2017 at 09:30:54PM +0000, Kieran Kunhya wrote:
> From d43c0366d4ba128772dc7909152f4e9635f269cb Mon Sep 17 00:00:00 2001
> From: Kieran Kunhya <kierank@obe.tv>
> Date: Fri, 5 May 2017 14:29:59 -0700
> Subject: [PATCH] libavcodec/h264_sei: Don't log random user data. This
>  prevents terminal junk.
> 
> ---
>  libavcodec/h264_sei.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/libavcodec/h264_sei.c b/libavcodec/h264_sei.c
> index a7e627eba3..c81ddfef4f 100644
> --- a/libavcodec/h264_sei.c
> +++ b/libavcodec/h264_sei.c
> @@ -257,9 +257,6 @@ static int
> decode_unregistered_user_data(H264SEIUnregistered *h, GetBitContext *
>      if (e == 1 && build == 1 && !strncmp(user_data+16, "x264 - core 0000",
> 16))
>          h->x264_build = 67;
> 
> -    if (strlen(user_data + 16) > 0)
> -        av_log(logctx, AV_LOG_DEBUG, "user data:\"%s\"\n", user_data + 16);

This is just at debug level.
Do you have an example (file/usecase) where this causes excessive
output ?


[...]

On 5/6/2017 1:37 PM, Michael Niedermayer wrote:
> On Fri, May 05, 2017 at 09:30:54PM +0000, Kieran Kunhya wrote:
>> From d43c0366d4ba128772dc7909152f4e9635f269cb Mon Sep 17 00:00:00 2001
>> From: Kieran Kunhya <kierank@obe.tv>
>> Date: Fri, 5 May 2017 14:29:59 -0700
>> Subject: [PATCH] libavcodec/h264_sei: Don't log random user data. This
>>  prevents terminal junk.
>>
>> ---
>>  libavcodec/h264_sei.c | 3 ---
>>  1 file changed, 3 deletions(-)
>>
>> diff --git a/libavcodec/h264_sei.c b/libavcodec/h264_sei.c
>> index a7e627eba3..c81ddfef4f 100644
>> --- a/libavcodec/h264_sei.c
>> +++ b/libavcodec/h264_sei.c
>> @@ -257,9 +257,6 @@ static int
>> decode_unregistered_user_data(H264SEIUnregistered *h, GetBitContext *
>>      if (e == 1 && build == 1 && !strncmp(user_data+16, "x264 - core 0000",
>> 16))
>>          h->x264_build = 67;
>>
>> -    if (strlen(user_data + 16) > 0)
>> -        av_log(logctx, AV_LOG_DEBUG, "user data:\"%s\"\n", user_data + 16);
> 
> This is just at debug level.
> Do you have an example (file/usecase) where this causes excessive
> output ?

It's not so much the amount to be printed as it's about not printing
random binary data. x264 is no the only encoder that writes this SEI,
and other encoders dump binary data in it. This can trash the terminal
displaying the log message.
The amount also matters, but that's solved by making sure
strlen(user_data + 16) is a sane value.

The proper change here would be to look at the uuid and print only those
known to store printable strings, and of course after making sure they
are in fact printable.
As of now, that means x264's uuid only.

On Sat, 6 May 2017 at 17:51 James Almer <jamrial@gmail.com> wrote:

> The proper change here would be to look at the uuid and print only those
> known to store printable strings, and of course after making sure they
> are in fact printable.
> As of now, that means x264's uuid only.
>

The UUID can be corrupt as well.
We shouldn't be printing random data from the bitstream to the users
terminal.

Kieran

On Sat, Nov 04, 2017 at 06:23:12PM +0000, Kieran Kunhya wrote:
> On Sat, 6 May 2017 at 17:51 James Almer <jamrial@gmail.com> wrote:
> 
> > The proper change here would be to look at the uuid and print only those
> > known to store printable strings, and of course after making sure they
> > are in fact printable.
> > As of now, that means x264's uuid only.
> >
> 
> The UUID can be corrupt as well.
> We shouldn't be printing random data from the bitstream to the users
> terminal.

i agree


[...]

On Sat, 4 Nov 2017 at 18:28 Michael Niedermayer <michael@niedermayer.cc>
wrote:

> > The UUID can be corrupt as well.
> > We shouldn't be printing random data from the bitstream to the users
> > terminal.
>
> i agree
>

Ok, I will push unless someone objects.

Kieran