Re: [FFmpeg-devel] [PATCH v2] avformat/westwood_aud: Adds PCM format demux.
2019-03-20 23:20 GMT+01:00, Aidan R : > From: ffmpeg-devel on behalf of Michael > Niedermayer > Sent: 20 March 2019 19:08 > To: FFmpeg development discussions and patches > Subject: Re: [FFmpeg-devel] [PATCH v2] avformat/westwood_aud: Adds PCM > format demux. > >> fails probetest: >> >> >> tools/probetest 256 4096 >> testing size=1 >> testing size=2 >> testing size=4 >> testing size=8 >> testing size=16 >> testing size=32 >> Failure of wsaud probing code with score=50 type=3 p=2D8 size=32 >> testing size=64 >> testing size=128 >> testing size=256 >> testing size=512 >> testing size=1024 >> testing size=2048 > > What is the appropriate way to resolve this if the format doesn't > provide enough information to identify it uniquely? Return > AVPROBE_SCORE_RETRY when we know we can't identify > a file with a high enough confidence? Another possibility would be to commit the patch without the change to the probe function (you can still force the format) and then think about the probe function independently. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v2] avformat/westwood_aud: Adds PCM format demux.
From: ffmpeg-devel on behalf of Michael Niedermayer Sent: 20 March 2019 19:08 To: FFmpeg development discussions and patches Subject: Re: [FFmpeg-devel] [PATCH v2] avformat/westwood_aud: Adds PCM format demux. > fails probetest: > > > tools/probetest 256 4096 > testing size=1 > testing size=2 > testing size=4 > testing size=8 > testing size=16 > testing size=32 > Failure of wsaud probing code with score=50 type=3 p=2D8 size=32 > testing size=64 > testing size=128 > testing size=256 > testing size=512 > testing size=1024 > testing size=2048 What is the appropriate way to resolve this if the format doesn't provide enough information to identify it uniquely? Return AVPROBE_SCORE_RETRY when we know we can't identify a file with a high enough confidence? -- Aidan Richmond ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v2] avformat/westwood_aud: Adds PCM format demux.
On Tue, Mar 19, 2019 at 10:53:11PM +, Aidan R wrote:
> PCM format AUD files are found in Westwood's Blade Runner game.
> ---
> libavformat/westwood_aud.c | 83
> --
> 1 file changed, 66 insertions(+), 17 deletions(-)
>
> diff --git a/libavformat/westwood_aud.c b/libavformat/westwood_aud.c
> index 9c2d35cb8a..b25d299bf0 100644
> --- a/libavformat/westwood_aud.c
> +++ b/libavformat/westwood_aud.c
> @@ -41,6 +41,12 @@
> #define AUD_HEADER_SIZE 12
> #define AUD_CHUNK_PREAMBLE_SIZE 8
> #define AUD_CHUNK_SIGNATURE 0xDEAF
> +#define AUD_PCM_CHUNK_SIZE 2048 /* arbitrary size to pull out of PCM data*/
> +
> +typedef struct AUDDemuxContext {
> +int size;
> +int pos;
> +} AUDDemuxContext;
>
> static int wsaud_probe(AVProbeData *p)
> {
> @@ -50,10 +56,10 @@ static int wsaud_probe(AVProbeData *p)
> * so perform sanity checks on various header parameters:
> * 8000 <= sample rate (16 bits) <= 48000 ==> 40001 acceptable numbers
> * flags <= 0x03 (2 LSBs are used) ==> 4 acceptable numbers
> - * compression type (8 bits) = 1 or 99 ==> 2 acceptable numbers
> + * compression type (8 bits) = 0, 1 or 99 ==> 3 acceptable numbers
> * first audio chunk signature (32 bits) ==> 1 acceptable number
> - * The number space contains 2^64 numbers. There are 40001 * 4 * 2 * 1 =
> - * 320008 acceptable number combinations.
> + * The number space contains 2^64 numbers. There are 40001 * 4 * 3 * 1 =
> + * 480012 acceptable number combinations.
> */
>
> if (p->buf_size < AUD_HEADER_SIZE + AUD_CHUNK_PREAMBLE_SIZE)
> @@ -69,13 +75,25 @@ static int wsaud_probe(AVProbeData *p)
> if (p->buf[10] & 0xFC)
> return 0;
>
> -if (p->buf[11] != 99 && p->buf[11] != 1)
> +/* valid format values are 99 == adpcm, 1 == snd1 and 0 == pcm */
> +if (p->buf[11] != 99 && p->buf[11] != 1 && p->buf[11] != 0)
> return 0;
>
> -/* read ahead to the first audio chunk and validate the first header
> signature */
> -if (AV_RL32(>buf[16]) != AUD_CHUNK_SIGNATURE)
> +/* read ahead to the first audio chunk and validate the first header
> + * signature pcm format does not use a chunk format, so don't check
> + * for that codec */
> +if (p->buf[11] != 0 && AV_RL32(>buf[16]) != AUD_CHUNK_SIGNATURE)
> return 0;
>
> +/* if we have pcm format then compressed size should equal
> + * uncompressed size */
> +if (p->buf[11] == 0) {
> +if (AV_RL32(>buf[2]) != AV_RL32(>buf[6]))
> +return 0;
> +if (AV_RL32(>buf[2]) + AUD_HEADER_SIZE < p->buf_size)
> +return 0;
> +}
> +
> /* return 1/2 certainty since this file check is a little sketchy */
> return AVPROBE_SCORE_EXTENSION;
> }
fails probetest:
tools/probetest 256 4096
testing size=1
testing size=2
testing size=4
testing size=8
testing size=16
testing size=32
Failure of wsaud probing code with score=50 type=3 p=2D8 size=32
testing size=64
testing size=128
testing size=256
testing size=512
testing size=1024
testing size=2048
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Observe your enemies, for they first find out your faults. -- Antisthenes
signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v2] avformat/westwood_aud: Adds PCM format demux.
ons 2019-03-20 klockan 15:56 + skrev Aidan R:
> tis 2019-03-19 klockan 22:53 + skrev Aidan R:
> > > @@ -130,20 +161,24 @@ static int wsaud_read_packet(AVFormatContext *s,
> > > AVPacket *pkt)
> > > {
> > > AVIOContext *pb = s->pb;
> > > +AUDDemuxContext *aud = s->priv_data;
> > > unsigned char preamble[AUD_CHUNK_PREAMBLE_SIZE];
> > > -unsigned int chunk_size;
> > > +unsigned int chunk_size, bytes_per_sample;
> > > int ret = 0;
> > > AVStream *st = s->streams[0];
> > >
> > > -if (avio_read(pb, preamble, AUD_CHUNK_PREAMBLE_SIZE) !=
> > > -AUD_CHUNK_PREAMBLE_SIZE)
> > > -return AVERROR(EIO);
> > > +/* AUD files don't store PCM audio in chunks */
> > > +if (st->codecpar->codec_id != AV_CODEC_ID_PCM_S16LE) {
> >
> > What about AV_CODEC_ID_PCM_U8?
>
> Good catch, I don't think any actually exist in the wild, but I have supported
> the possibility in wsaud_read_header and from looking at the audio engine it
> looks to support it in theory.
Given the recent 24-bit ZMBV discussion on this list, I think we should
not implement support for things which we don't have samples for. Using
it in the probe function is probably fine, but we should error out in
wsaud_read_header() until we have an 8-bit sample. The U8
implementation in this patch can be dummied out with #ifdefs until
then, for convenient un-#ifdefing
> >
> > A sample + FATE test for this would be nice
>
> Some guidance on where to start for that would be most welcome. I have short
> examples from Blade Runner that could be used but wouldn't distributing them
> violate copyright?
Cutting them down to a second or two should be fine I think. If we want
to be paranoid we could zero out the sample data as well, but I doubt
anyone cares. There's plenty of such samples in FATE. We could do this
in a separate thread, it doesn't have to hold up this patch
/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v2] avformat/westwood_aud: Adds PCM format demux.
tis 2019-03-19 klockan 22:53 + skrev Aidan R:
> @@ -69,13 +75,25 @@ static int wsaud_probe(AVProbeData *p)
>> if (p->buf[10] & 0xFC)
>> return 0;
>>
>> -if (p->buf[11] != 99 && p->buf[11] != 1)
>> +/* valid format values are 99 == adpcm, 1 == snd1 and 0 == pcm */
>> +if (p->buf[11] != 99 && p->buf[11] != 1 && p->buf[11] != 0)
>> return 0;
>>
>> -/* read ahead to the first audio chunk and validate the first header
>> signature */
>> -if (AV_RL32(>buf[16]) != AUD_CHUNK_SIGNATURE)
>> +/* read ahead to the first audio chunk and validate the first header
>> + * signature pcm format does not use a chunk format, so don't check
>
> Missing a period between "pcm" and "format"?
Missing after signature after re-reading it myself.
>
>> @@ -130,20 +161,24 @@ static int wsaud_read_packet(AVFormatContext *s,
>> AVPacket *pkt)
>> {
>> AVIOContext *pb = s->pb;
>> +AUDDemuxContext *aud = s->priv_data;
>> unsigned char preamble[AUD_CHUNK_PREAMBLE_SIZE];
>> -unsigned int chunk_size;
>> +unsigned int chunk_size, bytes_per_sample;
>> int ret = 0;
>> AVStream *st = s->streams[0];
>>
>> -if (avio_read(pb, preamble, AUD_CHUNK_PREAMBLE_SIZE) !=
>> -AUD_CHUNK_PREAMBLE_SIZE)
>> -return AVERROR(EIO);
>> +/* AUD files don't store PCM audio in chunks */
>> +if (st->codecpar->codec_id != AV_CODEC_ID_PCM_S16LE) {
>
>What about AV_CODEC_ID_PCM_U8?
Good catch, I don't think any actually exist in the wild, but I have supported
the possibility in wsaud_read_header and from looking at the audio engine it
looks to support it in theory.
>
>A sample + FATE test for this would be nice
Some guidance on where to start for that would be most welcome. I have short
examples from Blade Runner that could be used but wouldn't distributing them
violate copyright?
--
Aidan Richmond
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v2] avformat/westwood_aud: Adds PCM format demux.
tis 2019-03-19 klockan 22:53 + skrev Aidan R:
> @@ -69,13 +75,25 @@ static int wsaud_probe(AVProbeData *p)
> if (p->buf[10] & 0xFC)
> return 0;
>
> -if (p->buf[11] != 99 && p->buf[11] != 1)
> +/* valid format values are 99 == adpcm, 1 == snd1 and 0 == pcm */
> +if (p->buf[11] != 99 && p->buf[11] != 1 && p->buf[11] != 0)
> return 0;
>
> -/* read ahead to the first audio chunk and validate the first header
> signature */
> -if (AV_RL32(>buf[16]) != AUD_CHUNK_SIGNATURE)
> +/* read ahead to the first audio chunk and validate the first header
> + * signature pcm format does not use a chunk format, so don't check
Missing a period between "pcm" and "format"?
> @@ -130,20 +161,24 @@ static int wsaud_read_packet(AVFormatContext *s,
> AVPacket *pkt)
> {
> AVIOContext *pb = s->pb;
> +AUDDemuxContext *aud = s->priv_data;
> unsigned char preamble[AUD_CHUNK_PREAMBLE_SIZE];
> -unsigned int chunk_size;
> +unsigned int chunk_size, bytes_per_sample;
> int ret = 0;
> AVStream *st = s->streams[0];
>
> -if (avio_read(pb, preamble, AUD_CHUNK_PREAMBLE_SIZE) !=
> -AUD_CHUNK_PREAMBLE_SIZE)
> -return AVERROR(EIO);
> +/* AUD files don't store PCM audio in chunks */
> +if (st->codecpar->codec_id != AV_CODEC_ID_PCM_S16LE) {
What about AV_CODEC_ID_PCM_U8?
A sample + FATE test for this would be nice
/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
