[FFmpeg-devel] avformat:matroskadec: use a define to mark the EBML length is unknown

From: Steve Lhomme <robux4@ycbcr.xyz>

---
 libavformat/matroskadec.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

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

On Sat, Feb 23, 2019 at 11:14:33AM +0100, Steve Lhomme wrote:
> From: Steve Lhomme <robux4@ycbcr.xyz>
> 
> ---
>  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

[...]

On 2/23/19, Steve Lhomme <robUx4@ycbcr.xyz> wrote:
> From: Steve Lhomme <robux4@ycbcr.xyz>
>
> ---
>  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 = 0xffffffffffffffULL;
> +        *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 ==
> 0xffffffffffffff)
> +        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

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 <robux4@ycbcr.xyz>
>>
>> ---
>>   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 <robux4@ycbcr.xyz>
Co-authored-by: Dale Curtis <dalecurtis@chromium.org>

"

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 <robux4@ycbcr.xyz>
> >>
> >>---
> >>  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 <robux4@ycbcr.xyz>
> Co-authored-by: Dale Curtis <dalecurtis@chromium.org>
> 
> "

will apply with this as commit message

thanks

[...]