Message ID | 20200808133717.2311422-1-zane@zanevaniperen.com |
---|---|
State | Superseded |
Headers | show |
Series | [FFmpeg-devel,v2,1/4] avformat/argo_asf: don't check or probe file version | expand |
Context | Check | Description |
---|---|---|
andriy/default | pending | |
andriy/make | success | Make finished |
andriy/make_fate | success | Make fate finished |
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" >
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) {
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(-)