Re: [FFmpeg-devel] [PATCH]lavf/riff: Do not use a rogue twocc for adpcm_swf

2016-09-17 Thread Michael Niedermayer
On Sun, Sep 18, 2016 at 01:00:01AM +0200, Paul B Mahol wrote:
> On 9/17/16, Michael Niedermayer  wrote:
> > On Sat, Sep 17, 2016 at 03:06:57PM +0200, Carl Eugen Hoyos wrote:
> >> 2016-09-15 20:48 GMT+02:00 Michael Niedermayer :
> >> > On Thu, Sep 15, 2016 at 03:47:49PM +0200, Carl Eugen Hoyos wrote:
> >> >> 2016-09-15 15:42 GMT+02:00 Michael Niedermayer
> >> >> :
> >> >> >> -{ AV_CODEC_ID_ADPCM_SWF,   ('S' << 8) + 'F' },
> >> >> >>  /* HACK/FIXME: Does Vorbis in WAV/AVI have an (in)official ID?
> >> >> >> */
> >> >> >>  { AV_CODEC_ID_VORBIS,  ('V' << 8) + 'o' },
> >> >> >>  { AV_CODEC_ID_NONE,  0 },
> >> >> >
> >> >> > does this affect adpcm_swf in nut ?
> >> >>
> >> >> Yes, indeed.
> >> >>
> >> >> Is it possible to fix adpcm_swf in wav?
> >> >
> >> > maybe if block_align is set (and is constant)
> >>
> >> Attached fixes encoding, decoding still has the issue
> >> that the decoder doesn't like more than one packet.
> >> (Iiuc, it works if I limit the packet size in the wav demuxer.)
> >>
> >> Carl Eugen
> >
> >>  adpcmenc.c |1 +
> >>  1 file changed, 1 insertion(+)
> >> dccfa2d54e899c17dcb57b8e37658955dfa6ea9c
> >> 0001-lavc-adpcmenc-Set-block_align-for-adpcm_swf.patch
> >> From 168bc2f828ffd7f85c0db736e6097e4198854c5d Mon Sep 17 00:00:00 2001
> >> From: Carl Eugen Hoyos 
> >> Date: Sat, 17 Sep 2016 15:01:19 +0200
> >> Subject: [PATCH] lavc/adpcmenc: Set block_align for adpcm_swf.
> >>
> >> ---
> >>  libavcodec/adpcmenc.c |1 +
> >>  1 file changed, 1 insertion(+)
> >
> > LGTM
> >
> > can you add a fate test?
> 
> This patch is invalid. Block align should be set to what ADPCM_SWF
> uses in adpcm_encode_frame as pkt_size.

yes


> And ADPCM_SWF should not be special, it should use block_align as pkt_size.

yes

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

There will always be a question for which you do not know the correct answer.


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH]lavf/riff: Do not use a rogue twocc for adpcm_swf

2016-09-17 Thread Paul B Mahol
On 9/17/16, Michael Niedermayer  wrote:
> On Sat, Sep 17, 2016 at 03:06:57PM +0200, Carl Eugen Hoyos wrote:
>> 2016-09-15 20:48 GMT+02:00 Michael Niedermayer :
>> > On Thu, Sep 15, 2016 at 03:47:49PM +0200, Carl Eugen Hoyos wrote:
>> >> 2016-09-15 15:42 GMT+02:00 Michael Niedermayer
>> >> :
>> >> >> -{ AV_CODEC_ID_ADPCM_SWF,   ('S' << 8) + 'F' },
>> >> >>  /* HACK/FIXME: Does Vorbis in WAV/AVI have an (in)official ID?
>> >> >> */
>> >> >>  { AV_CODEC_ID_VORBIS,  ('V' << 8) + 'o' },
>> >> >>  { AV_CODEC_ID_NONE,  0 },
>> >> >
>> >> > does this affect adpcm_swf in nut ?
>> >>
>> >> Yes, indeed.
>> >>
>> >> Is it possible to fix adpcm_swf in wav?
>> >
>> > maybe if block_align is set (and is constant)
>>
>> Attached fixes encoding, decoding still has the issue
>> that the decoder doesn't like more than one packet.
>> (Iiuc, it works if I limit the packet size in the wav demuxer.)
>>
>> Carl Eugen
>
>>  adpcmenc.c |1 +
>>  1 file changed, 1 insertion(+)
>> dccfa2d54e899c17dcb57b8e37658955dfa6ea9c
>> 0001-lavc-adpcmenc-Set-block_align-for-adpcm_swf.patch
>> From 168bc2f828ffd7f85c0db736e6097e4198854c5d Mon Sep 17 00:00:00 2001
>> From: Carl Eugen Hoyos 
>> Date: Sat, 17 Sep 2016 15:01:19 +0200
>> Subject: [PATCH] lavc/adpcmenc: Set block_align for adpcm_swf.
>>
>> ---
>>  libavcodec/adpcmenc.c |1 +
>>  1 file changed, 1 insertion(+)
>
> LGTM
>
> can you add a fate test?

This patch is invalid. Block align should be set to what ADPCM_SWF
uses in adpcm_encode_frame as pkt_size.
And ADPCM_SWF should not be special, it should use block_align as pkt_size.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH]lavf/riff: Do not use a rogue twocc for adpcm_swf

2016-09-17 Thread Carl Eugen Hoyos
2016-09-17 18:26 GMT+02:00 Michael Niedermayer :

>> >> Is it possible to fix adpcm_swf in wav?
>> >
>> > maybe if block_align is set (and is constant)
>>
>> Attached fixes encoding, decoding still has the issue
>> that the decoder doesn't like more than one packet.
>> (Iiuc, it works if I limit the packet size in the wav demuxer.)

>>  adpcmenc.c |1 +
>>  1 file changed, 1 insertion(+)
>> dccfa2d54e899c17dcb57b8e37658955dfa6ea9c  
>> 0001-lavc-adpcmenc-Set-block_align-for-adpcm_swf.patch
>> From 168bc2f828ffd7f85c0db736e6097e4198854c5d Mon Sep 17 00:00:00 2001
>> From: Carl Eugen Hoyos 
>> Date: Sat, 17 Sep 2016 15:01:19 +0200
>> Subject: [PATCH] lavc/adpcmenc: Set block_align for adpcm_swf.
>>
>> ---
>>  libavcodec/adpcmenc.c |1 +
>>  1 file changed, 1 insertion(+)
>
> LGTM

It unfortunately still needs a decoder fix that I may not be
able to provide, I tested by reducing MAX_SIZE in wavdec.c

Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH]lavf/riff: Do not use a rogue twocc for adpcm_swf

2016-09-17 Thread Michael Niedermayer
On Sat, Sep 17, 2016 at 03:06:57PM +0200, Carl Eugen Hoyos wrote:
> 2016-09-15 20:48 GMT+02:00 Michael Niedermayer :
> > On Thu, Sep 15, 2016 at 03:47:49PM +0200, Carl Eugen Hoyos wrote:
> >> 2016-09-15 15:42 GMT+02:00 Michael Niedermayer :
> >> >> -{ AV_CODEC_ID_ADPCM_SWF,   ('S' << 8) + 'F' },
> >> >>  /* HACK/FIXME: Does Vorbis in WAV/AVI have an (in)official ID? */
> >> >>  { AV_CODEC_ID_VORBIS,  ('V' << 8) + 'o' },
> >> >>  { AV_CODEC_ID_NONE,  0 },
> >> >
> >> > does this affect adpcm_swf in nut ?
> >>
> >> Yes, indeed.
> >>
> >> Is it possible to fix adpcm_swf in wav?
> >
> > maybe if block_align is set (and is constant)
> 
> Attached fixes encoding, decoding still has the issue
> that the decoder doesn't like more than one packet.
> (Iiuc, it works if I limit the packet size in the wav demuxer.)
> 
> Carl Eugen

>  adpcmenc.c |1 +
>  1 file changed, 1 insertion(+)
> dccfa2d54e899c17dcb57b8e37658955dfa6ea9c  
> 0001-lavc-adpcmenc-Set-block_align-for-adpcm_swf.patch
> From 168bc2f828ffd7f85c0db736e6097e4198854c5d Mon Sep 17 00:00:00 2001
> From: Carl Eugen Hoyos 
> Date: Sat, 17 Sep 2016 15:01:19 +0200
> Subject: [PATCH] lavc/adpcmenc: Set block_align for adpcm_swf.
> 
> ---
>  libavcodec/adpcmenc.c |1 +
>  1 file changed, 1 insertion(+)

LGTM

can you add a fate test?

thx

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you drop bombs on a foreign country and kill a hundred thousand
innocent people, expect your government to call the consequence
"unprovoked inhuman terrorist attacks" and use it to justify dropping
more bombs and killing more people. The technology changed, the idea is old.


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH]lavf/riff: Do not use a rogue twocc for adpcm_swf

2016-09-17 Thread Carl Eugen Hoyos
2016-09-15 20:48 GMT+02:00 Michael Niedermayer :
> On Thu, Sep 15, 2016 at 03:47:49PM +0200, Carl Eugen Hoyos wrote:
>> 2016-09-15 15:42 GMT+02:00 Michael Niedermayer :
>> >> -{ AV_CODEC_ID_ADPCM_SWF,   ('S' << 8) + 'F' },
>> >>  /* HACK/FIXME: Does Vorbis in WAV/AVI have an (in)official ID? */
>> >>  { AV_CODEC_ID_VORBIS,  ('V' << 8) + 'o' },
>> >>  { AV_CODEC_ID_NONE,  0 },
>> >
>> > does this affect adpcm_swf in nut ?
>>
>> Yes, indeed.
>>
>> Is it possible to fix adpcm_swf in wav?
>
> maybe if block_align is set (and is constant)

Attached fixes encoding, decoding still has the issue
that the decoder doesn't like more than one packet.
(Iiuc, it works if I limit the packet size in the wav demuxer.)

Carl Eugen
From 168bc2f828ffd7f85c0db736e6097e4198854c5d Mon Sep 17 00:00:00 2001
From: Carl Eugen Hoyos 
Date: Sat, 17 Sep 2016 15:01:19 +0200
Subject: [PATCH] lavc/adpcmenc: Set block_align for adpcm_swf.

---
 libavcodec/adpcmenc.c |1 +
 1 file changed, 1 insertion(+)

diff --git a/libavcodec/adpcmenc.c b/libavcodec/adpcmenc.c
index 36974fd..8cf0d67 100644
--- a/libavcodec/adpcmenc.c
+++ b/libavcodec/adpcmenc.c
@@ -138,6 +138,7 @@ static av_cold int adpcm_encode_init(AVCodecContext *avctx)
 goto error;
 }
 avctx->frame_size = 512 * (avctx->sample_rate / 11025);
+avctx->block_align = (256 * avctx->channels * avctx->sample_rate / 11025) + 1 + 2 * avctx->channels;
 break;
 default:
 ret = AVERROR(EINVAL);
-- 
1.7.10.4

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH]lavf/riff: Do not use a rogue twocc for adpcm_swf

2016-09-15 Thread Michael Niedermayer
On Thu, Sep 15, 2016 at 03:47:49PM +0200, Carl Eugen Hoyos wrote:
> 2016-09-15 15:42 GMT+02:00 Michael Niedermayer :
> >> -{ AV_CODEC_ID_ADPCM_SWF,   ('S' << 8) + 'F' },
> >>  /* HACK/FIXME: Does Vorbis in WAV/AVI have an (in)official ID? */
> >>  { AV_CODEC_ID_VORBIS,  ('V' << 8) + 'o' },
> >>  { AV_CODEC_ID_NONE,  0 },
> >
> > does this affect adpcm_swf in nut ?
> 
> Yes, indeed.
> 
> Is it possible to fix adpcm_swf in wav?

maybe if block_align is set (and is constant)

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

It is what and why we do it that matters, not just one of them.


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH]lavf/riff: Do not use a rogue twocc for adpcm_swf

2016-09-15 Thread Carl Eugen Hoyos
2016-09-15 15:42 GMT+02:00 Michael Niedermayer :
>> -{ AV_CODEC_ID_ADPCM_SWF,   ('S' << 8) + 'F' },
>>  /* HACK/FIXME: Does Vorbis in WAV/AVI have an (in)official ID? */
>>  { AV_CODEC_ID_VORBIS,  ('V' << 8) + 'o' },
>>  { AV_CODEC_ID_NONE,  0 },
>
> does this affect adpcm_swf in nut ?

Yes, indeed.

Is it possible to fix adpcm_swf in wav?

Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH]lavf/riff: Do not use a rogue twocc for adpcm_swf

2016-09-15 Thread Michael Niedermayer
On Mon, Sep 12, 2016 at 12:52:49PM +0200, Carl Eugen Hoyos wrote:
> Hi!
> 
> Attached patch "fixes" ticket #5829, I am happy if a better 
> solution can be found.
> 
> Please comment, Carl Eugen

>  riff.c |1 -
>  1 file changed, 1 deletion(-)
> 6113805920f6fb418635029f2f600fcfe1c3fa88  
> 0001-lavf-riff-Do-not-use-a-rogue-twocc-for-adpcm_swf.patch
> From 69d62cac34908fb2a37e37ef1b03b565f2b4ae78 Mon Sep 17 00:00:00 2001
> From: Carl Eugen Hoyos 
> Date: Mon, 12 Sep 2016 12:42:33 +0200
> Subject: [PATCH] lavf/riff: Do not use a rogue twocc for adpcm_swf.
> 
> Fixes ticket #5829.
> ---
>  libavformat/riff.c |1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/libavformat/riff.c b/libavformat/riff.c
> index 72ad5d9..06f2996 100644
> --- a/libavformat/riff.c
> +++ b/libavformat/riff.c
> @@ -510,7 +510,6 @@ const AVCodecTag ff_codec_wav_tags[] = {
>  { AV_CODEC_ID_AAC, 0xA106 },
>  { AV_CODEC_ID_SPEEX,   0xA109 },
>  { AV_CODEC_ID_FLAC,0xF1AC },
> -{ AV_CODEC_ID_ADPCM_SWF,   ('S' << 8) + 'F' },
>  /* HACK/FIXME: Does Vorbis in WAV/AVI have an (in)official ID? */
>  { AV_CODEC_ID_VORBIS,  ('V' << 8) + 'o' },
>  { AV_CODEC_ID_NONE,  0 },

does this affect adpcm_swf in nut ? just guessing didnt try
aka should this maybe be moved into a nut specific table ?

[...]


-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Many that live deserve death. And some that die deserve life. Can you give
it to them? Then do not be too eager to deal out death in judgement. For
even the very wise cannot see all ends. -- Gandalf


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH]lavf/riff: Do not use a rogue twocc for adpcm_swf

2016-09-15 Thread Carl Eugen Hoyos
Hi!

2016-09-14 22:50 GMT+02:00 Ronald S. Bultje :
>
> On Wed, Sep 14, 2016 at 1:46 PM, Carl Eugen Hoyos wrote:
>
>> 2016-09-14 19:21 GMT+02:00 Ronald S. Bultje :

>> >> I wondered if somebody can easily fix muxing / demuxing which
>> >> could be considered a nicer solution.
>> >
>> > You mean remuxing (with -c:a copy) of swf files?
>>
>> There (also) is an encoder...
>
> I have literally no idea what you're trying to tell me.

Current FFmpeg supports decoding and encoding adpcm_swf.
It also supports demuxing and muxing adpcm_swf from and
to swf.
If muxing / demuxing adpcm_swf to and from wav could be
(easily) fixed, this may be a better patch than the one I provided.
(As in: More useful)

Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH]lavf/riff: Do not use a rogue twocc for adpcm_swf

2016-09-14 Thread Ronald S. Bultje
Hi,

On Wed, Sep 14, 2016 at 1:46 PM, Carl Eugen Hoyos 
wrote:

> 2016-09-14 19:21 GMT+02:00 Ronald S. Bultje :
>
> [...]
>
> >> I wondered if somebody can easily fix muxing / demuxing which
> >> could be considered a nicer solution.
> >
> > You mean remuxing (with -c:a copy) of swf files?
>
> There (also) is an encoder...


I have literally no idea what you're trying to tell me.

Ronald
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH]lavf/riff: Do not use a rogue twocc for adpcm_swf

2016-09-14 Thread Carl Eugen Hoyos
2016-09-14 19:21 GMT+02:00 Ronald S. Bultje :

[...]

>> I wondered if somebody can easily fix muxing / demuxing which
>> could be considered a nicer solution.
>
> You mean remuxing (with -c:a copy) of swf files?

There (also) is an encoder...

Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH]lavf/riff: Do not use a rogue twocc for adpcm_swf

2016-09-14 Thread Ronald S. Bultje
Hi,

On Wed, Sep 14, 2016 at 12:29 PM, Carl Eugen Hoyos 
wrote:

> Hi!
>
> 2016-09-14 18:05 GMT+02:00 Ronald S. Bultje :
> >
> > On Wed, Sep 14, 2016 at 12:01 PM, Carl Eugen Hoyos wrote:
> >
> >> 2016-09-12 12:52 GMT+02:00 Carl Eugen Hoyos :
> >>
> >> > Attached patch "fixes" ticket #5829, I am happy if a better
> >> > solution can be found.
>
> > What makes you believe this is not the right solution?
>
> > As in: why was the rogue twocc ever added?
>
> I don't think we will be able to find out (or actually: I don't want to
> share my suspicion).
>

I would love to hear.


> I wondered if somebody can easily fix muxing / demuxing which
> could be considered a nicer solution.


You mean remuxing (with -c:a copy) of swf files?

Ronald
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH]lavf/riff: Do not use a rogue twocc for adpcm_swf

2016-09-14 Thread Carl Eugen Hoyos
Hi!

2016-09-14 18:05 GMT+02:00 Ronald S. Bultje :
>
> On Wed, Sep 14, 2016 at 12:01 PM, Carl Eugen Hoyos wrote:
>
>> 2016-09-12 12:52 GMT+02:00 Carl Eugen Hoyos :
>>
>> > Attached patch "fixes" ticket #5829, I am happy if a better
>> > solution can be found.

> What makes you believe this is not the right solution?

> As in: why was the rogue twocc ever added?

I don't think we will be able to find out (or actually: I don't want to
share my suspicion).

I wondered if somebody can easily fix muxing / demuxing which
could be considered a nicer solution.

Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH]lavf/riff: Do not use a rogue twocc for adpcm_swf

2016-09-14 Thread Ronald S. Bultje
Hi,

On Wed, Sep 14, 2016 at 12:01 PM, Carl Eugen Hoyos 
wrote:

> 2016-09-12 12:52 GMT+02:00 Carl Eugen Hoyos :
>
> > Attached patch "fixes" ticket #5829, I am happy if a better
> > solution can be found.
>
> Ping.


What makes you believe this is not the right solution? As in: why was the
rogue twocc ever added?

IMO patch is OK, and other rogue twoccs should be removed also, if they
allow creating invalid and unplayable files (like vorbis in wav).

Ronald
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH]lavf/riff: Do not use a rogue twocc for adpcm_swf

2016-09-14 Thread Carl Eugen Hoyos
2016-09-12 12:52 GMT+02:00 Carl Eugen Hoyos :

> Attached patch "fixes" ticket #5829, I am happy if a better
> solution can be found.

Ping.

Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH]lavf/riff: Do not use a rogue twocc for adpcm_swf

2016-09-12 Thread Carl Eugen Hoyos
Hi!

Attached patch "fixes" ticket #5829, I am happy if a better 
solution can be found.

Please comment, Carl Eugen
From 69d62cac34908fb2a37e37ef1b03b565f2b4ae78 Mon Sep 17 00:00:00 2001
From: Carl Eugen Hoyos 
Date: Mon, 12 Sep 2016 12:42:33 +0200
Subject: [PATCH] lavf/riff: Do not use a rogue twocc for adpcm_swf.

Fixes ticket #5829.
---
 libavformat/riff.c |1 -
 1 file changed, 1 deletion(-)

diff --git a/libavformat/riff.c b/libavformat/riff.c
index 72ad5d9..06f2996 100644
--- a/libavformat/riff.c
+++ b/libavformat/riff.c
@@ -510,7 +510,6 @@ const AVCodecTag ff_codec_wav_tags[] = {
 { AV_CODEC_ID_AAC, 0xA106 },
 { AV_CODEC_ID_SPEEX,   0xA109 },
 { AV_CODEC_ID_FLAC,0xF1AC },
-{ AV_CODEC_ID_ADPCM_SWF,   ('S' << 8) + 'F' },
 /* HACK/FIXME: Does Vorbis in WAV/AVI have an (in)official ID? */
 { AV_CODEC_ID_VORBIS,  ('V' << 8) + 'o' },
 { AV_CODEC_ID_NONE,  0 },
-- 
1.7.10.4

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel