Re: [FFmpeg-devel] [PATCH v2 1/7] avutil/uuid: add utility library for manipulating UUIDs as specified in RFC 4122
On Wed, May 11, 2022 at 8:02 AM Zane van Iperen wrote:
>
>
>
> On 10/5/22 23:18, Andreas Rheinhardt wrote:
>
> >> +int av_uuid_parse_range(const char *in_start, const char *in_end, AVUUID
> >> uu)
> >> +{
> >> +int i;
> >> +const char *cp;
> >> +char buf[3];
> >> +
> >> +if ((in_end - in_start) != 36)
> >> +return -1;
> >> +
> >> +for (i = 0, cp = in_start; i < 36; i++, cp++) {
> >> +if (i == 8 || i == 13 || i == 18 || i == 23) {
> >> +if (*cp == '-')
> >> +continue;
> >> +return AVERROR(EINVAL);
> >> +}
> >> +
> >> +if (!av_isxdigit(*cp))
> >> +return AVERROR(EINVAL);
> >> +}
> >> +
> >> +buf[2] = '\0';
> >> +for (i = 0, cp = in_start; i < 16; i++) {
> >> +if (i == 4 || i == 6 || i == 8 || i == 10)
> >> +cp++;
> >> +
> >> +buf[0] = *cp++;
> >> +buf[1] = *cp++;
> >> +
> >> +errno = 0;
> >> +uu[i] = strtoul(buf, NULL, 16);
> >> +if (errno)
> >> +return AVERROR(errno);
> >
> > How could this ever happen given that you have already checked that the
> > buffer only contains hex digits? And isn't using strtoul a bit overkill
> > anyway? I'd just check and parse this stuff in one loop.
> >
>
> Yeah, good point. It's based off libuuid's, which has some time/clock uuid
> handling
> between the two loops. I'll tidy it up in the next few days... hopefully...
>
> >> +/**
> >> + * @file
> >> + * UUID parsing and serialization utilities.
> >> + * The library treat the UUID as an opaque sequence of 16 unsigned bytes,
> > ^
> > s
>
> Fixed.
>
> >> +/**
> >> + * Parses a string representation of a UUID formatted according to IETF
> >> RFC 4122
> >> + * into an AVUUID. The parsing is case-insensitive. The string must be 37
> >> + * characters long, including the terminating NULL character.
> >
> > NUL, NULL is for pointers.
> >
>
> Changed.
>
> >> +/**
> >> + * Parses a string representation of a UUID formatted according to IETF
> >> RFC 4122
> >> + * into an AVUUID. The parsing is case-insensitive. The string must
> >> consist of
> >> + * 36 characters, i.e. `in_end - in_start == 36`
> >> + *
> >> + * @param[in] in_start Pointer to the first character of the string
> >> representation
> >> + * @param[in] in_end Pointer to the character after the last character
> >> of the
> >> + * string representation. That memory location is
> >> never
> >> + * accessed
> >> + * @param[out] uu AVUUID
> >> + * @return A non-zero value in case of an error.
> >> + */
> >> +int av_uuid_parse_range(const char *in_start, const char *in_end, AVUUID
> >> uu);
> >
> > This sounds like in_end is actually redundant. Does retaining it improve
> > extensibility?
> >
>
> I believe so. The main difference is av_uuid_parse_range() can handle non
> NUL-termiated
> strings. I can just remove the entire last sentence (or change "must" to
> "should").
I have modified the documentation in v3 of the patchset to emphasize
that `in_end - in_start != 36` results in an error.
>
> >> +
> >> +/**
> >> + * Serializes a AVUUID into a string representation according to IETF RFC
> >> 4122.
> >> + * The string is lowercase and always 37 characters long, including the
> >> + * terminating NULL character.
> >> + *
> >> + * @param[in] uu AVUUID
> >> + * @param[out] out Pointer to a array of no less than 37 characters.
> >^
> >n
> >
>
> Fixed.
>
>
> >> +/**
> >> + * Sets a UUID to nil
> >> + *
> >> + * @param[in,out] uu UUID to be set to nil
> >> + */
> >> +void av_uuid_nil_set(AVUUID uu);
> >
> > Why are these three functions not static inline? Exporting them seems
> > like a waste.
> >
>
> No particular reason, it's easy enough to change.
>
> ___
> 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 v2 1/7] avutil/uuid: add utility library for manipulating UUIDs as specified in RFC 4122
On Wed, May 11, 2022 at 8:03 AM Zane van Iperen wrote: > > > > On 11/5/22 21:53, Anton Khirnov wrote: > > Quoting Zane van Iperen (2022-04-24 12:14:03) > >> +void av_uuid_nil_set(AVUUID uu) > > ^^^ > > sounds weird > > > > av_uuid_zero()? > > av_uuid_reset()? > > > > Good point, `av_uuid_zero()` has a nice ring to it. I think we should keep `nil` in the name since the _nil UUID_ has a special meaning in RFC 4122. have proposed av_uuid_nil() in v3 of the patchset. > ___ > 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 v2 1/7] avutil/uuid: add utility library for manipulating UUIDs as specified in RFC 4122
On 11/5/22 21:53, Anton Khirnov wrote: Quoting Zane van Iperen (2022-04-24 12:14:03) +void av_uuid_nil_set(AVUUID uu) ^^^ sounds weird av_uuid_zero()? av_uuid_reset()? Good point, `av_uuid_zero()` has a nice ring to it. ___ 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 v2 1/7] avutil/uuid: add utility library for manipulating UUIDs as specified in RFC 4122
On 10/5/22 23:18, Andreas Rheinhardt wrote:
+int av_uuid_parse_range(const char *in_start, const char *in_end, AVUUID uu)
+{
+int i;
+const char *cp;
+char buf[3];
+
+if ((in_end - in_start) != 36)
+return -1;
+
+for (i = 0, cp = in_start; i < 36; i++, cp++) {
+if (i == 8 || i == 13 || i == 18 || i == 23) {
+if (*cp == '-')
+continue;
+return AVERROR(EINVAL);
+}
+
+if (!av_isxdigit(*cp))
+return AVERROR(EINVAL);
+}
+
+buf[2] = '\0';
+for (i = 0, cp = in_start; i < 16; i++) {
+if (i == 4 || i == 6 || i == 8 || i == 10)
+cp++;
+
+buf[0] = *cp++;
+buf[1] = *cp++;
+
+errno = 0;
+uu[i] = strtoul(buf, NULL, 16);
+if (errno)
+return AVERROR(errno);
How could this ever happen given that you have already checked that the
buffer only contains hex digits? And isn't using strtoul a bit overkill
anyway? I'd just check and parse this stuff in one loop.
Yeah, good point. It's based off libuuid's, which has some time/clock uuid
handling
between the two loops. I'll tidy it up in the next few days... hopefully...
+/**
+ * @file
+ * UUID parsing and serialization utilities.
+ * The library treat the UUID as an opaque sequence of 16 unsigned bytes,
^
s
Fixed.
+/**
+ * Parses a string representation of a UUID formatted according to IETF RFC
4122
+ * into an AVUUID. The parsing is case-insensitive. The string must be 37
+ * characters long, including the terminating NULL character.
NUL, NULL is for pointers.
Changed.
+/**
+ * Parses a string representation of a UUID formatted according to IETF RFC
4122
+ * into an AVUUID. The parsing is case-insensitive. The string must consist of
+ * 36 characters, i.e. `in_end - in_start == 36`
+ *
+ * @param[in] in_start Pointer to the first character of the string
representation
+ * @param[in] in_end Pointer to the character after the last character of
the
+ * string representation. That memory location is never
+ * accessed
+ * @param[out] uu AVUUID
+ * @return A non-zero value in case of an error.
+ */
+int av_uuid_parse_range(const char *in_start, const char *in_end, AVUUID uu);
This sounds like in_end is actually redundant. Does retaining it improve
extensibility?
I believe so. The main difference is av_uuid_parse_range() can handle non
NUL-termiated
strings. I can just remove the entire last sentence (or change "must" to
"should").
+
+/**
+ * Serializes a AVUUID into a string representation according to IETF RFC 4122.
+ * The string is lowercase and always 37 characters long, including the
+ * terminating NULL character.
+ *
+ * @param[in] uu AVUUID
+ * @param[out] out Pointer to a array of no less than 37 characters.
^
n
Fixed.
+/**
+ * Sets a UUID to nil
+ *
+ * @param[in,out] uu UUID to be set to nil
+ */
+void av_uuid_nil_set(AVUUID uu);
Why are these three functions not static inline? Exporting them seems
like a waste.
No particular reason, it's easy enough to change.
___
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 v2 1/7] avutil/uuid: add utility library for manipulating UUIDs as specified in RFC 4122
Quoting Zane van Iperen (2022-04-24 12:14:03) > +void av_uuid_nil_set(AVUUID uu) ^^^ sounds weird av_uuid_zero()? av_uuid_reset()? -- Anton Khirnov ___ 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 v2 1/7] avutil/uuid: add utility library for manipulating UUIDs as specified in RFC 4122
Zane van Iperen:
> Co-authored-by: Pierre-Anthony Lemieux
> Signed-off-by: Zane van Iperen
> ---
> libavutil/Makefile | 2 +
> libavutil/uuid.c | 155 +
> libavutil/uuid.h | 137 +++
> 3 files changed, 294 insertions(+)
> create mode 100644 libavutil/uuid.c
> create mode 100644 libavutil/uuid.h
>
> diff --git a/libavutil/Makefile b/libavutil/Makefile
> index 81df3b0640..29742668b8 100644
> --- a/libavutil/Makefile
> +++ b/libavutil/Makefile
> @@ -82,6 +82,7 @@ HEADERS = adler32.h
> \
>timestamp.h \
>tree.h\
>twofish.h \
> + uuid.h\
>version.h \
>video_enc_params.h\
>xtea.h\
> @@ -174,6 +175,7 @@ OBJS = adler32.o
> \
> tx_float.o \
> tx_double.o \
> tx_int32.o \
> + uuid.o \
> video_enc_params.o \
> film_grain_params.o \
>
> diff --git a/libavutil/uuid.c b/libavutil/uuid.c
> new file mode 100644
> index 00..6cf7a20c23
> --- /dev/null
> +++ b/libavutil/uuid.c
> @@ -0,0 +1,155 @@
> +/*
> + * Copyright (c) 2022 Pierre-Anthony Lemieux
> + *Zane van Iperen
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
> USA
> + */
> +
> +/*
> + * Copyright (C) 1996, 1997 Theodore Ts'o.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + * 1. Redistributions of source code must retain the above copyright
> + *notice, and the entire permission notice in its entirety,
> + *including the disclaimer of warranties.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + *notice, this list of conditions and the following disclaimer in the
> + *documentation and/or other materials provided with the distribution.
> + * 3. The name of the author may not be used to endorse or promote
> + *products derived from this software without specific prior
> + *written permission.
> + *
> + * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED
> + * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
> + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE, ALL OF
> + * WHICH ARE HEREBY DISCLAIMED. IN NO EVENT SHALL THE AUTHOR BE
> + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT
> + * OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
> + * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
> + * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
> + * USE OF THIS SOFTWARE, EVEN IF NOT ADVISED OF THE POSSIBILITY OF SUCH
> + * DAMAGE.
> + */
> +
> +/**
> + * @file
> + * UUID parsing and serialization utilities.
> + * The library treat the UUID as an opaque sequence of 16 unsigned bytes,
> + * i.e. ignoring the internal layout of the UUID, which depends on the type
> + * of the UUID.
> + *
> + * @author Pierre-Anthony Lemieux
> + * @author Zane van Iperen
> + */
> +
> +#include "uuid.h"
> +#include "error.h"
> +#include "avstring.h"
> +#include
> +#include
> +
> +int av_uuid_parse(const char *in, AVUUID uu)
> +{
> +if (strlen(in) != 36)
> +return AVERROR(EINVAL);
> +
> +return av_uuid
[FFmpeg-devel] [PATCH v2 1/7] avutil/uuid: add utility library for manipulating UUIDs as specified in RFC 4122
Co-authored-by: Pierre-Anthony Lemieux
Signed-off-by: Zane van Iperen
---
libavutil/Makefile | 2 +
libavutil/uuid.c | 155 +
libavutil/uuid.h | 137 +++
3 files changed, 294 insertions(+)
create mode 100644 libavutil/uuid.c
create mode 100644 libavutil/uuid.h
diff --git a/libavutil/Makefile b/libavutil/Makefile
index 81df3b0640..29742668b8 100644
--- a/libavutil/Makefile
+++ b/libavutil/Makefile
@@ -82,6 +82,7 @@ HEADERS = adler32.h
\
timestamp.h \
tree.h\
twofish.h \
+ uuid.h\
version.h \
video_enc_params.h\
xtea.h\
@@ -174,6 +175,7 @@ OBJS = adler32.o
\
tx_float.o \
tx_double.o \
tx_int32.o \
+ uuid.o \
video_enc_params.o \
film_grain_params.o \
diff --git a/libavutil/uuid.c b/libavutil/uuid.c
new file mode 100644
index 00..6cf7a20c23
--- /dev/null
+++ b/libavutil/uuid.c
@@ -0,0 +1,155 @@
+/*
+ * Copyright (c) 2022 Pierre-Anthony Lemieux
+ *Zane van Iperen
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/*
+ * Copyright (C) 1996, 1997 Theodore Ts'o.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *notice, and the entire permission notice in its entirety,
+ *including the disclaimer of warranties.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *notice, this list of conditions and the following disclaimer in the
+ *documentation and/or other materials provided with the distribution.
+ * 3. The name of the author may not be used to endorse or promote
+ *products derived from this software without specific prior
+ *written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED
+ * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE, ALL OF
+ * WHICH ARE HEREBY DISCLAIMED. IN NO EVENT SHALL THE AUTHOR BE
+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT
+ * OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
+ * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
+ * USE OF THIS SOFTWARE, EVEN IF NOT ADVISED OF THE POSSIBILITY OF SUCH
+ * DAMAGE.
+ */
+
+/**
+ * @file
+ * UUID parsing and serialization utilities.
+ * The library treat the UUID as an opaque sequence of 16 unsigned bytes,
+ * i.e. ignoring the internal layout of the UUID, which depends on the type
+ * of the UUID.
+ *
+ * @author Pierre-Anthony Lemieux
+ * @author Zane van Iperen
+ */
+
+#include "uuid.h"
+#include "error.h"
+#include "avstring.h"
+#include
+#include
+
+int av_uuid_parse(const char *in, AVUUID uu)
+{
+if (strlen(in) != 36)
+return AVERROR(EINVAL);
+
+return av_uuid_parse_range(in, in + 36, uu);
+}
+
+int av_uuid_parse_range(const char *in_start, const char *in_end, AVUUID uu)
+{
+int i;
+const char *cp;
+char buf[3];
+
+if ((in_end - in_start) != 36)
+return -1;
+
+for (i = 0,
