Re: [FFmpeg-devel] [PATCH] avformat:matroskadec: use a define to mark the EBML length is unknown
On Sun, Feb 24, 2019 at 09:23:48AM +0100, Steve Lhomme wrote: > On 23/02/2019 19:37, Michael Niedermayer wrote: > >On Sat, Feb 23, 2019 at 11:14:33AM +0100, Steve Lhomme wrote: > >>From: Steve Lhomme > >> > >>--- > >> libavformat/matroskadec.c | 12 +++- > >> 1 file changed, 7 insertions(+), 5 deletions(-) > >I think the commit message is a bit terse. This is not just a cosmetic > >change (which one might think from reading just the commit message and > >not looking at the change or related mails) > > > >Can you provide a more verbose commit message ? > >ill apply it with that > > > >thanks > > > >[...] > > How about this ? > > " > avformat:matroskadec: use a define to mark the EBML length is unknown > > Unifying the way the EBML unknown length is signaled, rather than using two > incompatible values. UINT64_MAX cannot be read as a valid EBML length with > the > current code. > > Co-authored-by: Steve Lhomme > Co-authored-by: Dale Curtis > > " will apply with this as commit message thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB I have never wished to cater to the crowd; for what I know they do not approve, and what they approve I do not know. -- Epicurus signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat:matroskadec: use a define to mark the EBML length is unknown
On 23/02/2019 19:37, Michael Niedermayer wrote: On Sat, Feb 23, 2019 at 11:14:33AM +0100, Steve Lhomme wrote: From: Steve Lhomme --- libavformat/matroskadec.c | 12 +++- 1 file changed, 7 insertions(+), 5 deletions(-) I think the commit message is a bit terse. This is not just a cosmetic change (which one might think from reading just the commit message and not looking at the change or related mails) Can you provide a more verbose commit message ? ill apply it with that thanks [...] How about this ? " avformat:matroskadec: use a define to mark the EBML length is unknown Unifying the way the EBML unknown length is signaled, rather than using two incompatible values. UINT64_MAX cannot be read as a valid EBML length with the current code. Co-authored-by: Steve Lhomme Co-authored-by: Dale Curtis " ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat:matroskadec: use a define to mark the EBML length is unknown
On 2/23/19, Steve Lhomme wrote:
> From: Steve Lhomme
>
> ---
> libavformat/matroskadec.c | 12 +++-
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> index 5aa8a105dc..0e3a6890c1 100644
> --- a/libavformat/matroskadec.c
> +++ b/libavformat/matroskadec.c
> @@ -68,6 +68,8 @@
>
> #include "qtpalette.h"
>
> +#define EBML_UNKNOWN_LENGTH UINT64_MAX /* EBML unknown length, in uint64_t
> */
> +
> typedef enum {
> EBML_NONE,
> EBML_UINT,
> @@ -869,7 +871,7 @@ static int ebml_read_length(MatroskaDemuxContext
> *matroska, AVIOContext *pb,
> {
> int res = ebml_read_num(matroska, pb, 8, number);
> if (res > 0 && *number + 1 == 1ULL << (7 * res))
> -*number = 0xffULL;
> +*number = EBML_UNKNOWN_LENGTH;
> return res;
> }
>
> @@ -1049,7 +1051,7 @@ static int ebml_parse_id(MatroskaDemuxContext
> *matroska, EbmlSyntax *syntax,
> break;
> if (!syntax[i].id && id == MATROSKA_ID_CLUSTER &&
> matroska->num_levels > 0 &&
> -matroska->levels[matroska->num_levels - 1].length ==
> 0xff)
> +matroska->levels[matroska->num_levels - 1].length ==
> EBML_UNKNOWN_LENGTH)
> return 0; // we reached the end of an unknown size cluster
> if (!syntax[i].id && id != EBML_ID_VOID && id != EBML_ID_CRC32) {
> av_log(matroska->ctx, AV_LOG_DEBUG, "Unknown entry 0x%"PRIX32"\n",
> id);
> @@ -1201,7 +1203,7 @@ static int ebml_parse_elem(MatroskaDemuxContext
> *matroska,
> MatroskaLevel *level = &matroska->levels[matroska->num_levels -
> 1];
> AVIOContext *pb = matroska->ctx->pb;
> int64_t pos = avio_tell(pb);
> -if (level->length != (uint64_t) -1 &&
> +if (level->length != EBML_UNKNOWN_LENGTH &&
> (pos + length) > (level->start + level->length)) {
> av_log(matroska->ctx, AV_LOG_ERROR,
> "Invalid length 0x%"PRIx64" > 0x%"PRIx64" in
> parent\n",
> @@ -1610,7 +1612,7 @@ static int
> matroska_parse_seekhead_entry(MatroskaDemuxContext *matroska,
> ret = AVERROR_INVALIDDATA;
> } else {
> level.start = 0;
> -level.length = (uint64_t) -1;
> +level.length = EBML_UNKNOWN_LENGTH;
> matroska->levels[matroska->num_levels] = level;
> matroska->num_levels++;
> matroska->current_id = 0;
> @@ -1620,7 +1622,7 @@ static int
> matroska_parse_seekhead_entry(MatroskaDemuxContext *matroska,
> /* remove dummy level */
> while (matroska->num_levels) {
> uint64_t length =
> matroska->levels[--matroska->num_levels].length;
> -if (length == (uint64_t) -1)
> +if (length == EBML_UNKNOWN_LENGTH)
> break;
> }
> }
> --
> 2.18.0
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
lgtm
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat:matroskadec: use a define to mark the EBML length is unknown
On Sat, Feb 23, 2019 at 11:14:33AM +0100, Steve Lhomme wrote: > From: Steve Lhomme > > --- > libavformat/matroskadec.c | 12 +++- > 1 file changed, 7 insertions(+), 5 deletions(-) I think the commit message is a bit terse. This is not just a cosmetic change (which one might think from reading just the commit message and not looking at the change or related mails) Can you provide a more verbose commit message ? ill apply it with that thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Elect your leaders based on what they did after the last election, not based on what they say before an election. signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat:matroskadec: use a define to mark the EBML length is unknown
Steve Lhomme wrote: > libavformat/matroskadec.c | 12 +++- > 1 file changed, 7 insertions(+), 5 deletions(-) > >diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c >index 5aa8a105dc..0e3a6890c1 100644 Should be fine. Best regards, Reto ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avformat:matroskadec: use a define to mark the EBML length is unknown
From: Steve Lhomme
---
libavformat/matroskadec.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 5aa8a105dc..0e3a6890c1 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -68,6 +68,8 @@
#include "qtpalette.h"
+#define EBML_UNKNOWN_LENGTH UINT64_MAX /* EBML unknown length, in uint64_t */
+
typedef enum {
EBML_NONE,
EBML_UINT,
@@ -869,7 +871,7 @@ static int ebml_read_length(MatroskaDemuxContext *matroska,
AVIOContext *pb,
{
int res = ebml_read_num(matroska, pb, 8, number);
if (res > 0 && *number + 1 == 1ULL << (7 * res))
-*number = 0xffULL;
+*number = EBML_UNKNOWN_LENGTH;
return res;
}
@@ -1049,7 +1051,7 @@ static int ebml_parse_id(MatroskaDemuxContext *matroska,
EbmlSyntax *syntax,
break;
if (!syntax[i].id && id == MATROSKA_ID_CLUSTER &&
matroska->num_levels > 0 &&
-matroska->levels[matroska->num_levels - 1].length == 0xff)
+matroska->levels[matroska->num_levels - 1].length ==
EBML_UNKNOWN_LENGTH)
return 0; // we reached the end of an unknown size cluster
if (!syntax[i].id && id != EBML_ID_VOID && id != EBML_ID_CRC32) {
av_log(matroska->ctx, AV_LOG_DEBUG, "Unknown entry 0x%"PRIX32"\n", id);
@@ -1201,7 +1203,7 @@ static int ebml_parse_elem(MatroskaDemuxContext *matroska,
MatroskaLevel *level = &matroska->levels[matroska->num_levels - 1];
AVIOContext *pb = matroska->ctx->pb;
int64_t pos = avio_tell(pb);
-if (level->length != (uint64_t) -1 &&
+if (level->length != EBML_UNKNOWN_LENGTH &&
(pos + length) > (level->start + level->length)) {
av_log(matroska->ctx, AV_LOG_ERROR,
"Invalid length 0x%"PRIx64" > 0x%"PRIx64" in parent\n",
@@ -1610,7 +1612,7 @@ static int
matroska_parse_seekhead_entry(MatroskaDemuxContext *matroska,
ret = AVERROR_INVALIDDATA;
} else {
level.start = 0;
-level.length = (uint64_t) -1;
+level.length = EBML_UNKNOWN_LENGTH;
matroska->levels[matroska->num_levels] = level;
matroska->num_levels++;
matroska->current_id = 0;
@@ -1620,7 +1622,7 @@ static int
matroska_parse_seekhead_entry(MatroskaDemuxContext *matroska,
/* remove dummy level */
while (matroska->num_levels) {
uint64_t length =
matroska->levels[--matroska->num_levels].length;
-if (length == (uint64_t) -1)
+if (length == EBML_UNKNOWN_LENGTH)
break;
}
}
--
2.18.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
