Re: [FFmpeg-devel] [PATCH v2 5/7] avcodec/cbs_sei: refactor to use avutil/uuid

[Pierre-Anthony Lemieux <pal@sandflow.com>]

On Sat, Apr 30, 2022 at 12:26 PM Mark Thompson <sw@jkqxz.net> wrote:
>
> On 30/04/2022 18:53, Pierre-Anthony Lemieux wrote:
> > On Sat, Apr 30, 2022 at 10:31 AM Mark Thompson <sw@jkqxz.net> wrote:
> >>
> >> On 24/04/2022 11:14, Zane van Iperen wrote:
> >>> From: Pierre-Anthony Lemieux <pal@palemieux.com>
> >>>
> >>> ---
> >>>    libavcodec/cbs_sei.h           | 3 ++-
> >>>    libavcodec/vaapi_encode_h264.c | 8 ++++----
> >>>    2 files changed, 6 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/libavcodec/cbs_sei.h b/libavcodec/cbs_sei.h
> >>> index c7a7a95be0..67c6e6cbbd 100644
> >>> --- a/libavcodec/cbs_sei.h
> >>> +++ b/libavcodec/cbs_sei.h
> >>> @@ -23,6 +23,7 @@
> >>>    #include <stdint.h>
> >>>
> >>>    #include "libavutil/buffer.h"
> >>> +#include "libavutil/uuid.h"
> >>>
> >>>    #include "cbs.h"
> >>>    #include "sei.h"
> >>> @@ -41,7 +42,7 @@ typedef struct SEIRawUserDataRegistered {
> >>>    } SEIRawUserDataRegistered;
> >>>
> >>>    typedef struct SEIRawUserDataUnregistered {
> >>> -    uint8_t      uuid_iso_iec_11578[16];
> >>> +    AVUUID      uuid_iso_iec_11578;
> >>>        uint8_t     *data;
> >>>        AVBufferRef *data_ref;
> >>>        size_t       data_length;
> >>
> >> This feels like a step backwards?  The syntax template files are explicitly relying on this being uint8_t[16], so giving it a different name is confusing.
> >
> > What/where are the syntax template files?
>
> <http://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavcodec/cbs_sei_syntax_template.c#l87>
>
> (It's included twice by cbs_h2645.c for read/write with different macro setups.)

Ok. Thanks. Are you concerned that the following line assumes that
uuid_iso_iec_11578 is uint8_t[16] instead of being the opaque AVUUID?

us(8, uuid_iso_iec_11578[i], 0x00, 0xff, 1, i);

Did I get this right? If so, couple of options come to mind:

(a) revert the change
(b) define a macro to access individual bytes of AVUUID, thereby
keeping AVUUID opaque
(c) not handle AVUUID as opaque, but instead always as uint8_t[16]

Maybe additional options exist.

I do not have a definitive opinion. Some folks expressed strong
interest in having a consistent scheme for manipulating UUIDs.

>
> - Mark
> _______________________________________________
> 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".