[FFmpeg-devel,v2,1/4] avformat/argo_asf: don't check or probe file version

All files I've seen are identical, irregardless of version.
Until shown otherwise, assume unknown ones are too.

Signed-off-by: Zane van Iperen <zane@zanevaniperen.com>
---
 libavformat/argo_asf.c | 37 +++++++------------------------------
 1 file changed, 7 insertions(+), 30 deletions(-)

This is really bad practice.

On 8/8/20, Zane van Iperen <zane@zanevaniperen.com> wrote:
> All files I've seen are identical, irregardless of version.
> Until shown otherwise, assume unknown ones are too.
>
> Signed-off-by: Zane van Iperen <zane@zanevaniperen.com>
> ---
>  libavformat/argo_asf.c | 37 +++++++------------------------------
>  1 file changed, 7 insertions(+), 30 deletions(-)
>
> diff --git a/libavformat/argo_asf.c b/libavformat/argo_asf.c
> index 671b7482f9..beec46a0d4 100644
> --- a/libavformat/argo_asf.c
> +++ b/libavformat/argo_asf.c
> @@ -29,6 +29,12 @@
>  #define ASF_CHUNK_HEADER_SIZE   20
>  #define ASF_SAMPLE_COUNT        32
>
> +/*
> + * Known versions:
> + * 1.1: The sample files in /game-formats/brender/part2.zip
> + * 1.2: Croc! Legend of the Gobbos
> + * 2.1: Croc 2
> + */
>  typedef struct ArgoASFFileHeader {
>      uint32_t    magic;          /*< Magic Number, {'A', 'S', 'F', '\0'} */
>      uint16_t    version_major;  /*< File Major Version. */
> @@ -85,33 +91,11 @@ static void
> argo_asf_parse_chunk_header(ArgoASFChunkHeader *hdr, const uint8_t *
>      hdr->flags          = AV_RL32(buf + 16);
>  }
>
> -/*
> - * Known versions:
> - * 1.1: The sample files in /game-formats/brender/part2.zip
> - * 1.2: Croc! Legend of the Gobbos
> - * 2.1: Croc 2
> - */
> -static int argo_asf_is_known_version(const ArgoASFFileHeader *hdr)
> -{
> -    return (hdr->version_major == 1 && hdr->version_minor == 1) ||
> -           (hdr->version_major == 1 && hdr->version_minor == 2) ||
> -           (hdr->version_major == 2 && hdr->version_minor == 1);
> -}
> -
>  static int argo_asf_probe(const AVProbeData *p)
>  {
> -    ArgoASFFileHeader hdr;
> -
> -    av_assert0(AVPROBE_PADDING_SIZE >= ASF_FILE_HEADER_SIZE);
> -
> -    argo_asf_parse_file_header(&hdr, p->buf);
> -
> -    if (hdr.magic != ASF_TAG)
> +    if (AV_RL32(p->buf) != ASF_TAG)
>          return 0;
>
> -    if (!argo_asf_is_known_version(&hdr))
> -        return AVPROBE_SCORE_EXTENSION / 2;
> -
>      return AVPROBE_SCORE_EXTENSION + 1;
>  }
>
> @@ -133,13 +117,6 @@ static int argo_asf_read_header(AVFormatContext *s)
>
>      argo_asf_parse_file_header(&asf->fhdr, buf);
>
> -    if (!argo_asf_is_known_version(&asf->fhdr)) {
> -        avpriv_request_sample(s, "Version %hu.%hu",
> -            asf->fhdr.version_major, asf->fhdr.version_minor
> -        );
> -        return AVERROR_PATCHWELCOME;
> -    }
> -
>      if (asf->fhdr.num_chunks == 0) {
>          return AVERROR_INVALIDDATA;
>      } else if (asf->fhdr.num_chunks > 1) {
> --
> 2.25.1
>
>
> _______________________________________________
> 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".

On Sun, 9 Aug 2020 09:29:16 +0200
"Paul B Mahol" <onemda@gmail.com> wrote:

> 
> This is really bad practice.
> 

Usually I'd agree. However (and I've just checked this) all file
versions are identical.

Since this is an Argonaut Games format, I checked the version of all the
ASF files in their most recent games. The only two versions found were
1.2 and 2.1, both of which are already supported.

Because this format is looong-dead, I see no harm in ignoring file
version (especially if the muxer allows specifying arbitrary versions).

I shall update the commit message accordingly.

$ for i in ~/Desktop/staging/asf_music/*/*; do ./asfdump.py $i | jq ._version; done | sort -u
"1.2"
"2.1"

Am 9. August 2020 14:33:26 MESZ schrieb Zane van Iperen <zane@zanevaniperen.com>:
>On Sun, 9 Aug 2020 09:29:16 +0200
>"Paul B Mahol" <onemda@gmail.com> wrote:
>
>> 
>> This is really bad practice.
>> 
>
>Usually I'd agree. However (and I've just checked this) all file
>versions are identical.
>
>Since this is an Argonaut Games format, I checked the version of all
>the
>ASF files in their most recent games. The only two versions found were
>1.2 and 2.1, both of which are already supported.
>
>Because this format is looong-dead, I see no harm in ignoring file
>version (especially if the muxer allows specifying arbitrary versions).

IMHO ignoring version in read_header might be OK in this case.

Another way would be erroring out and say use ignore_version option. Don't think it's necessary here, but would maybe be a bit more honest and we could ask for samples at the same time.

Probably more problematic is the part completely removing the check for known version from the probe function. Shouldn't that stay?


  Alexander

>I shall update the commit message accordingly.
>
>$ for i in ~/Desktop/staging/asf_music/*/*; do ./asfdump.py $i | jq
>._version; done | sort -u
>"1.2"
>"2.1"
>