On Wed, 8 Feb 2017, Anton Khirnov wrote:
Quoting Diego Biurrun (2017-02-07 17:39:38)
On Tue, Feb 07, 2017 at 02:15:55PM +0100, Alexandra Hájková wrote:
> the ASF specific Format Data size. Fixes video decoding problem
> part of the bug 1020.
See the other review for log message hints.
> --- a/libavformat/asfdec.c
> +++ b/libavformat/asfdec.c
> @@ -691,16 +691,18 @@ static int asf_read_properties(AVFormatContext *s,
const GUIDParseTable *g)
>
> static int parse_video_info(AVIOContext *pb, AVStream *st)
> {
> + uint16_t size_;
> + uint32_t size;
> unsigned int tag;
Please use descriptive variable names.
> --- a/libavformat/riff.h
> +++ b/libavformat/riff.h
> @@ -43,7 +43,7 @@ void ff_end_tag(AVIOContext *pb, int64_t start);
> * bits_per_encoded_sample fields. Does not read extradata.
> * @return codec tag
> */
> -int ff_get_bmp_header(AVIOContext *pb, AVStream *st);
> +int ff_get_bmp_header(AVIOContext *pb, AVStream *st, uint32_t *size);
The documentation is now incomplete.
Why uint32_t? Sizes should have size_t as type.
Your size_t obsession is getting out of control. It makes sense for some
places, but not all of them. As is rather obvious from the patch, the
size is stored as uint32 in the bitstream, therefore it makes sense to
store it as uint32 in code.
To elaborate on this: If you make such fields a size_t, you end up parsing
files differently depending on whether you're using a 32 or 64 bit build.
For such values; if they are supposed to be over 32 bits, use an uint64_t
and it will work consistently and correctly on both. If it isn't supposed
to be that, use an uint32_t.
// Martin
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
