Re: [FFmpeg-devel] [PATCH v1 3/5] avfilter/vf_showinfo: display user data unregistered message

2019-12-18 Thread Limin Wang
On Wed, Dec 18, 2019 at 11:22:05PM +0100, Michael Niedermayer wrote:
> On Wed, Dec 18, 2019 at 08:55:04AM +0800, Limin Wang wrote:
> > On Tue, Dec 17, 2019 at 10:22:47PM +0100, Michael Niedermayer wrote:
> > > On Tue, Dec 17, 2019 at 06:22:15PM +0800, lance.lmw...@gmail.com wrote:
> > > > From: Limin Wang 
> > > > 
> > > > Signed-off-by: Limin Wang 
> > > > ---
> > > >  libavfilter/vf_showinfo.c | 20 
> > > >  1 file changed, 20 insertions(+)
> > > > 
> > > > diff --git a/libavfilter/vf_showinfo.c b/libavfilter/vf_showinfo.c
> > > > index 31f6b32aa4..0d227983c2 100644
> > > > --- a/libavfilter/vf_showinfo.c
> > > > +++ b/libavfilter/vf_showinfo.c
> > > > @@ -169,6 +169,23 @@ static void 
> > > > dump_content_light_metadata(AVFilterContext *ctx, AVFrameSideData *s
> > > > metadata->MaxCLL, metadata->MaxFALL);
> > > >  }
> > > >  
> > > > +static void dump_user_data_unregistered_metadata(AVFilterContext *ctx, 
> > > > AVFrameSideData *sd)
> > > > +{
> > > > +const int uuid_size = 16;
> > > > +
> > > > +if (sd->size < uuid_size) {
> > > > +av_log(ctx, AV_LOG_ERROR, "invalid data");
> > > > +return;
> > > > +}
> > > 
> > > The need for a UUID (of 16bytes) is not described in the text describing 
> > > this side data type
> > By the specs:
> > user_data_unregistered( payloadSize ) { C Descriptor
> > uuid_iso_iec_11578  5 u(128)
> > for( i = 16; i < payloadSize; i++ )
> > user_data_payload_byte  5 b(8)
> > }
> 
> ive been inprecise
> what i have meant was this:
> @@ -179,6 +179,13 @@ enum AVFrameSideDataType {
>   * array element is implied by AVFrameSideData.size / 
> AVRegionOfInterest.self_size.
>   */  
>   
>  AV_FRAME_DATA_REGIONS_OF_INTEREST,
> +
> +/**
> + * User data unregistered metadata associated with a video frame.
> + * This user data payload is stored as uint8_t in AVFrameSideData.data.
> + * The number of bytes of user data is AVFrameSideData.size.

Got it, I'll add the one more line description for the need for the UUID.
* The user data consists of 16 bytes UUID and the other parts are 
* real user data .

> + */
> +AV_FRAME_DATA_USER_DATA_UNREGISTERED,
>  };
> 
>  enum AVActiveFormatDescription {
> 
> [...]
> -- 
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> 
> Rewriting code that is poorly written but fully understood is good.
> Rewriting code that one doesnt understand is a sign that one is less smart
> then the original author, trying to rewrite it will not make it better.



> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v1 3/5] avfilter/vf_showinfo: display user data unregistered message

2019-12-18 Thread Michael Niedermayer
On Wed, Dec 18, 2019 at 08:55:04AM +0800, Limin Wang wrote:
> On Tue, Dec 17, 2019 at 10:22:47PM +0100, Michael Niedermayer wrote:
> > On Tue, Dec 17, 2019 at 06:22:15PM +0800, lance.lmw...@gmail.com wrote:
> > > From: Limin Wang 
> > > 
> > > Signed-off-by: Limin Wang 
> > > ---
> > >  libavfilter/vf_showinfo.c | 20 
> > >  1 file changed, 20 insertions(+)
> > > 
> > > diff --git a/libavfilter/vf_showinfo.c b/libavfilter/vf_showinfo.c
> > > index 31f6b32aa4..0d227983c2 100644
> > > --- a/libavfilter/vf_showinfo.c
> > > +++ b/libavfilter/vf_showinfo.c
> > > @@ -169,6 +169,23 @@ static void 
> > > dump_content_light_metadata(AVFilterContext *ctx, AVFrameSideData *s
> > > metadata->MaxCLL, metadata->MaxFALL);
> > >  }
> > >  
> > > +static void dump_user_data_unregistered_metadata(AVFilterContext *ctx, 
> > > AVFrameSideData *sd)
> > > +{
> > > +const int uuid_size = 16;
> > > +
> > > +if (sd->size < uuid_size) {
> > > +av_log(ctx, AV_LOG_ERROR, "invalid data");
> > > +return;
> > > +}
> > 
> > The need for a UUID (of 16bytes) is not described in the text describing 
> > this side data type
> By the specs:
> user_data_unregistered( payloadSize ) { C Descriptor
> uuid_iso_iec_11578  5 u(128)
> for( i = 16; i < payloadSize; i++ )
> user_data_payload_byte  5 b(8)
> }

ive been inprecise
what i have meant was this:
@@ -179,6 +179,13 @@ enum AVFrameSideDataType {
  * array element is implied by AVFrameSideData.size / 
AVRegionOfInterest.self_size.
  */

 AV_FRAME_DATA_REGIONS_OF_INTEREST,
+
+/**
+ * User data unregistered metadata associated with a video frame.
+ * This user data payload is stored as uint8_t in AVFrameSideData.data.
+ * The number of bytes of user data is AVFrameSideData.size.
+ */
+AV_FRAME_DATA_USER_DATA_UNREGISTERED,
 };

 enum AVActiveFormatDescription {

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Rewriting code that is poorly written but fully understood is good.
Rewriting code that one doesnt understand is a sign that one is less smart
then the original author, trying to rewrite it will not make it better.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v1 3/5] avfilter/vf_showinfo: display user data unregistered message

2019-12-17 Thread Limin Wang
On Tue, Dec 17, 2019 at 10:22:47PM +0100, Michael Niedermayer wrote:
> On Tue, Dec 17, 2019 at 06:22:15PM +0800, lance.lmw...@gmail.com wrote:
> > From: Limin Wang 
> > 
> > Signed-off-by: Limin Wang 
> > ---
> >  libavfilter/vf_showinfo.c | 20 
> >  1 file changed, 20 insertions(+)
> > 
> > diff --git a/libavfilter/vf_showinfo.c b/libavfilter/vf_showinfo.c
> > index 31f6b32aa4..0d227983c2 100644
> > --- a/libavfilter/vf_showinfo.c
> > +++ b/libavfilter/vf_showinfo.c
> > @@ -169,6 +169,23 @@ static void 
> > dump_content_light_metadata(AVFilterContext *ctx, AVFrameSideData *s
> > metadata->MaxCLL, metadata->MaxFALL);
> >  }
> >  
> > +static void dump_user_data_unregistered_metadata(AVFilterContext *ctx, 
> > AVFrameSideData *sd)
> > +{
> > +const int uuid_size = 16;
> > +
> > +if (sd->size < uuid_size) {
> > +av_log(ctx, AV_LOG_ERROR, "invalid data");
> > +return;
> > +}
> 
> The need for a UUID (of 16bytes) is not described in the text describing 
> this side data type
By the specs:
user_data_unregistered( payloadSize ) { C Descriptor
uuid_iso_iec_11578  5 u(128)
for( i = 16; i < payloadSize; i++ )
user_data_payload_byte  5 b(8)
}

So it's need to make sure the uuid is 16bytes, now the user buffer = uuid + 
user_data, for the uuid need keep same when copy to output.


> 
> [...]
> 
> -- 
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> 
> Modern terrorism, a quick summary: Need oil, start war with country that
> has oil, kill hundread thousand in war. Let country fall into chaos,
> be surprised about raise of fundamantalists. Drop more bombs, kill more
> people, be surprised about them taking revenge and drop even more bombs
> and strip your own citizens of their rights and freedoms. to be continued



> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v1 3/5] avfilter/vf_showinfo: display user data unregistered message

2019-12-17 Thread Limin Wang
On Tue, Dec 17, 2019 at 06:18:55PM -0300, James Almer wrote:
> On 12/17/2019 7:22 AM, lance.lmw...@gmail.com wrote:
> > From: Limin Wang 
> > 
> > Signed-off-by: Limin Wang 
> > ---
> >  libavfilter/vf_showinfo.c | 20 
> >  1 file changed, 20 insertions(+)
> > 
> > diff --git a/libavfilter/vf_showinfo.c b/libavfilter/vf_showinfo.c
> > index 31f6b32aa4..0d227983c2 100644
> > --- a/libavfilter/vf_showinfo.c
> > +++ b/libavfilter/vf_showinfo.c
> > @@ -169,6 +169,23 @@ static void 
> > dump_content_light_metadata(AVFilterContext *ctx, AVFrameSideData *s
> > metadata->MaxCLL, metadata->MaxFALL);
> >  }
> >  
> > +static void dump_user_data_unregistered_metadata(AVFilterContext *ctx, 
> > AVFrameSideData *sd)
> > +{
> > +const int uuid_size = 16;
> > +
> > +if (sd->size < uuid_size) {
> > +av_log(ctx, AV_LOG_ERROR, "invalid data");
> > +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, "%x", sd->data[i]);
> > +av_log(ctx, AV_LOG_INFO, "\n");
> > +av_log(ctx, AV_LOG_INFO, "User Data=%s", sd->data + uuid_size);
> 
> I recall we used to print any user unregistered data SEI in debug mode
> but eventually stopped since it presented a risk. We can't just blindly
> print whatever is contained here. It should at least be checked that
> it's actually printable characters and not random binary data.

yes, I'll add safe check for string is ascii or not.

> 
> > +}
> > +
> >  static void dump_color_property(AVFilterContext *ctx, AVFrame *frame)
> >  {
> >  const char *color_range_str = 
> > av_color_range_name(frame->color_range);
> > @@ -319,6 +336,9 @@ static int filter_frame(AVFilterLink *inlink, AVFrame 
> > *frame)
> >  av_log(ctx, AV_LOG_INFO, "GOP timecode - %s", tcbuf);
> >  break;
> >  }
> > +case AV_FRAME_DATA_USER_DATA_UNREGISTERED:
> > +dump_user_data_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@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v1 3/5] avfilter/vf_showinfo: display user data unregistered message

2019-12-17 Thread Michael Niedermayer
On Tue, Dec 17, 2019 at 06:22:15PM +0800, lance.lmw...@gmail.com wrote:
> From: Limin Wang 
> 
> Signed-off-by: Limin Wang 
> ---
>  libavfilter/vf_showinfo.c | 20 
>  1 file changed, 20 insertions(+)
> 
> diff --git a/libavfilter/vf_showinfo.c b/libavfilter/vf_showinfo.c
> index 31f6b32aa4..0d227983c2 100644
> --- a/libavfilter/vf_showinfo.c
> +++ b/libavfilter/vf_showinfo.c
> @@ -169,6 +169,23 @@ static void dump_content_light_metadata(AVFilterContext 
> *ctx, AVFrameSideData *s
> metadata->MaxCLL, metadata->MaxFALL);
>  }
>  
> +static void dump_user_data_unregistered_metadata(AVFilterContext *ctx, 
> AVFrameSideData *sd)
> +{
> +const int uuid_size = 16;
> +
> +if (sd->size < uuid_size) {
> +av_log(ctx, AV_LOG_ERROR, "invalid data");
> +return;
> +}

The need for a UUID (of 16bytes) is not described in the text describing 
this side data type

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Modern terrorism, a quick summary: Need oil, start war with country that
has oil, kill hundread thousand in war. Let country fall into chaos,
be surprised about raise of fundamantalists. Drop more bombs, kill more
people, be surprised about them taking revenge and drop even more bombs
and strip your own citizens of their rights and freedoms. to be continued


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v1 3/5] avfilter/vf_showinfo: display user data unregistered message

2019-12-17 Thread James Almer
On 12/17/2019 7:22 AM, lance.lmw...@gmail.com wrote:
> From: Limin Wang 
> 
> Signed-off-by: Limin Wang 
> ---
>  libavfilter/vf_showinfo.c | 20 
>  1 file changed, 20 insertions(+)
> 
> diff --git a/libavfilter/vf_showinfo.c b/libavfilter/vf_showinfo.c
> index 31f6b32aa4..0d227983c2 100644
> --- a/libavfilter/vf_showinfo.c
> +++ b/libavfilter/vf_showinfo.c
> @@ -169,6 +169,23 @@ static void dump_content_light_metadata(AVFilterContext 
> *ctx, AVFrameSideData *s
> metadata->MaxCLL, metadata->MaxFALL);
>  }
>  
> +static void dump_user_data_unregistered_metadata(AVFilterContext *ctx, 
> AVFrameSideData *sd)
> +{
> +const int uuid_size = 16;
> +
> +if (sd->size < uuid_size) {
> +av_log(ctx, AV_LOG_ERROR, "invalid data");
> +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, "%x", sd->data[i]);
> +av_log(ctx, AV_LOG_INFO, "\n");
> +av_log(ctx, AV_LOG_INFO, "User Data=%s", sd->data + uuid_size);

I recall we used to print any user unregistered data SEI in debug mode
but eventually stopped since it presented a risk. We can't just blindly
print whatever is contained here. It should at least be checked that
it's actually printable characters and not random binary data.

> +}
> +
>  static void dump_color_property(AVFilterContext *ctx, AVFrame *frame)
>  {
>  const char *color_range_str = 
> av_color_range_name(frame->color_range);
> @@ -319,6 +336,9 @@ static int filter_frame(AVFilterLink *inlink, AVFrame 
> *frame)
>  av_log(ctx, AV_LOG_INFO, "GOP timecode - %s", tcbuf);
>  break;
>  }
> +case AV_FRAME_DATA_USER_DATA_UNREGISTERED:
> +dump_user_data_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@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v1 3/5] avfilter/vf_showinfo: display user data unregistered message

2019-12-17 Thread lance . lmwang
From: Limin Wang 

Signed-off-by: Limin Wang 
---
 libavfilter/vf_showinfo.c | 20 
 1 file changed, 20 insertions(+)

diff --git a/libavfilter/vf_showinfo.c b/libavfilter/vf_showinfo.c
index 31f6b32aa4..0d227983c2 100644
--- a/libavfilter/vf_showinfo.c
+++ b/libavfilter/vf_showinfo.c
@@ -169,6 +169,23 @@ static void dump_content_light_metadata(AVFilterContext 
*ctx, AVFrameSideData *s
metadata->MaxCLL, metadata->MaxFALL);
 }
 
+static void dump_user_data_unregistered_metadata(AVFilterContext *ctx, 
AVFrameSideData *sd)
+{
+const int uuid_size = 16;
+
+if (sd->size < uuid_size) {
+av_log(ctx, AV_LOG_ERROR, "invalid data");
+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, "%x", sd->data[i]);
+av_log(ctx, AV_LOG_INFO, "\n");
+av_log(ctx, AV_LOG_INFO, "User Data=%s", sd->data + uuid_size);
+}
+
 static void dump_color_property(AVFilterContext *ctx, AVFrame *frame)
 {
 const char *color_range_str = av_color_range_name(frame->color_range);
@@ -319,6 +336,9 @@ static int filter_frame(AVFilterLink *inlink, AVFrame 
*frame)
 av_log(ctx, AV_LOG_INFO, "GOP timecode - %s", tcbuf);
 break;
 }
+case AV_FRAME_DATA_USER_DATA_UNREGISTERED:
+dump_user_data_unregistered_metadata(ctx, sd);
+break;
 default:
 av_log(ctx, AV_LOG_WARNING, "unknown side data type %d (%d bytes)",
sd->type, sd->size);
-- 
2.21.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".