| Message ID | 20180114130605.uabevtlu3kimi2pk@gnuage.org |
|---|---|
| State | New |
| Headers | show |
On 14 January 2018 at 13:06, Aurelien Jacobs <aurel@gnuage.org> wrote: > On Tue, Jan 09, 2018 at 02:21:02PM +0000, Rostislav Pehlivanov wrote: > > On 9 January 2018 at 14:07, Rostislav Pehlivanov <atomnuker@gmail.com> > > wrote: > > > > > > > > > > > On 9 January 2018 at 09:00, Hendrik Leppkes <h.leppkes@gmail.com> > wrote: > > > > > >> On Tue, Jan 9, 2018 at 9:33 AM, Hendrik Leppkes <h.leppkes@gmail.com> > > >> wrote: > > >> > On Tue, Jan 9, 2018 at 5:07 AM, Rostislav Pehlivanov > > >> > <atomnuker@gmail.com> wrote: > > >> >> > > >> >>> Anyway, all this discussion is moot as Hendrik pointed out that > > >> profile > > >> >>> can't be set outside of lavc to determine a decoder behavior. > > >> >>> > > >> >> > > >> >> What, based on a comment in lavc? Comments there describe the api > but > > >> they > > >> >> rarely define it. We're free to adjust them if need be and in this > > >> case, > > >> >> since the codec profile may not be set, the API user is forced to > deal > > >> with > > >> >> the lack of such set. Hence, we can clarify that it may be set by > lavf > > >> as > > >> >> well as lavc as well as not being set at all. And the decoder may > use > > >> it. > > >> >> I maintain that adding a new codec ID for this is unacceptable. > > >> >> > > >> > > > >> > We already have established methods to select codec sub-variants, we > > >> > don't need to invent a new one just because you feel like it. > > >> > > > >> > > >> On that note, we also have cases where the same codec has different > > >> codec ids based on popular known names. > > >> For example wmv3/vc1 - wmv3 is simple/main profile, vc1 is advanced > > >> profile, different codec ids, same implementation. > > >> > > >> Re-defining public API is what is unacceptable, it requires every > > >> caller of lavc to suddenly start handling one more field for no real > > >> reason. > > >> > > > > > > Then its a good thing I suggested something that doesn't involve having > > > every caller of lavc to handle another field. > > > > > > > > > > > >> Either use a separate codec ID if there is sufficient reason for that > > >> (mostly driven by external factors, if its handled as different codecs > > >> everywhere else, not only in marketing but actual (reference) > > >> implementations), or use a codec_tag if one is available (the codec id > > >> field from a2dp for example) > > > > > > I'd be fine with using codec tags, but not with codec IDs. > > > > Though for encoding using profiles would be best. > > Well, here is an updated patch which uses codec tags for the decoder and > profile for the encoder. > > I still maintain that this solution is worse than using separate > codec IDs. > It is worse for developper / maintainer as it adds a bit of compexity to > the code. > It is worse for user as it requires adding a mandatory parameter for > encoding to aptX HD: > ffmpeg -i sample.wav -profile:a 1 sample.aptxhd > As opposed to having a mandatory parameter for encoder like -c:a aptx_hd? Perhaps we should encode everything to aptx hd by default to not have to mess about by confusing users with these codecs or profiles things. Without this -profile parameter, the encoding will just fail but user > will have to guess how to fix it. > Here's how you could fix it: don't have separate muxers for aptx and aptxhd. Make the aptx muxer handle both .aptx and .aptxhd extensions and the codec tags. There's no difference in how either are encoded since its all raw. Sure, it prevents strictly mandating that an aptxhd extension will always carry aptxhd bitstreams, but what do you expect, its a raw format and there are no guarantees that extensions always mandate what's in a file. That's why we probe. I'm fine with either having a separate muxer or a common one with multiple extensions, so I don't mind things as they are here. > And the reported stream is much less explicit: > Stream #0:0: Audio: aptx ([36][0][215][0] / 0xD70024), 48000 Hz, 2 > channels, s32p > vs. > Stream #0:0: Audio: aptx_hd, 48000 Hz, 2 channels, s32p > Profiles are still explicit "Stream #0:0: Audio: aac (LC), 44100 Hz, stereo, fltp (16 bit), 128 kb/s" vs "Stream #0:0: Audio: aac (Main), 44100 Hz, stereo, fltp (16 bit), 128 kb/s" > So for the good of users and maintainers, I suggest to follow the > wmv3/vc1 example, that is to acknowledge that aptX and aptX HD are > commonly used as 2 separate codecs (search for > libaptX-1.0.0-rel-Android21-ARMv7A.so and > libaptXHD-1.0.0-rel-Android21-ARMv7A.so for example) and use the > original patch with 2 codec IDs. > Ah, android, a shining beacon of well-written, small, generic code, always happy to reuse code and other people's work. No, wait, the other way around. Anyway, code-wise, there's one or two minor issues: +#define A2DP_VENDOR_ID_APT 0x0000004F > +#define A2DP_APT_CODEC_ID_APTX 0x0001 > + > +#define A2DP_VENDOR_ID_QUALCOMM 0x000000D7 > +#define A2DP_QUALCOMM_CODEC_ID_APTX_HD 0x0024 > You have 3 copies of those in both this patch and the muxer/demuxer one. Make libavcodec/aptx.h and put those there, then include it from lavf. +OBJS-$(CONFIG_APTX_HD_DECODER) += aptx.o > +OBJS-$(CONFIG_APTX_HD_ENCODER) += aptx.o > No point in having those flags now that there's one encoder/decoder handling both. - .video_codec = AV_CODEC_ID_NONE, > Why? That aside, patches look good to me.
2018-01-14 14:06 GMT+01:00 Aurelien Jacobs <aurel@gnuage.org>: > Well, here is an updated patch which uses codec tags for the decoder and > profile for the encoder. Sorry but I object to this patch: We should not invent codec_tags. Carl Eugen
On Sun, Jan 14, 2018 at 05:19:12PM +0000, Rostislav Pehlivanov wrote: > On 14 January 2018 at 13:06, Aurelien Jacobs <aurel@gnuage.org> wrote: > > > On Tue, Jan 09, 2018 at 02:21:02PM +0000, Rostislav Pehlivanov wrote: > > > On 9 January 2018 at 14:07, Rostislav Pehlivanov <atomnuker@gmail.com> > > > wrote: > > > > > > > > > > > > > > > On 9 January 2018 at 09:00, Hendrik Leppkes <h.leppkes@gmail.com> > > wrote: > > > > > > > >> On Tue, Jan 9, 2018 at 9:33 AM, Hendrik Leppkes <h.leppkes@gmail.com> > > > >> wrote: > > > >> > On Tue, Jan 9, 2018 at 5:07 AM, Rostislav Pehlivanov > > > >> > <atomnuker@gmail.com> wrote: > > > >> >> > > > >> >>> Anyway, all this discussion is moot as Hendrik pointed out that > > > >> profile > > > >> >>> can't be set outside of lavc to determine a decoder behavior. > > > >> >>> > > > >> >> > > > >> >> What, based on a comment in lavc? Comments there describe the api > > but > > > >> they > > > >> >> rarely define it. We're free to adjust them if need be and in this > > > >> case, > > > >> >> since the codec profile may not be set, the API user is forced to > > deal > > > >> with > > > >> >> the lack of such set. Hence, we can clarify that it may be set by > > lavf > > > >> as > > > >> >> well as lavc as well as not being set at all. And the decoder may > > use > > > >> it. > > > >> >> I maintain that adding a new codec ID for this is unacceptable. > > > >> >> > > > >> > > > > >> > We already have established methods to select codec sub-variants, we > > > >> > don't need to invent a new one just because you feel like it. > > > >> > > > > >> > > > >> On that note, we also have cases where the same codec has different > > > >> codec ids based on popular known names. > > > >> For example wmv3/vc1 - wmv3 is simple/main profile, vc1 is advanced > > > >> profile, different codec ids, same implementation. > > > >> > > > >> Re-defining public API is what is unacceptable, it requires every > > > >> caller of lavc to suddenly start handling one more field for no real > > > >> reason. > > > >> > > > > > > > > Then its a good thing I suggested something that doesn't involve having > > > > every caller of lavc to handle another field. > > > > > > > > > > > > > > > >> Either use a separate codec ID if there is sufficient reason for that > > > >> (mostly driven by external factors, if its handled as different codecs > > > >> everywhere else, not only in marketing but actual (reference) > > > >> implementations), or use a codec_tag if one is available (the codec id > > > >> field from a2dp for example) > > > > > > > > I'd be fine with using codec tags, but not with codec IDs. > > > > > > Though for encoding using profiles would be best. > > > > Well, here is an updated patch which uses codec tags for the decoder and > > profile for the encoder. > > > > I still maintain that this solution is worse than using separate > > codec IDs. > > It is worse for developper / maintainer as it adds a bit of compexity to > > the code. > > > > It is worse for user as it requires adding a mandatory parameter for > > encoding to aptX HD: > > ffmpeg -i sample.wav -profile:a 1 sample.aptxhd > > > > As opposed to having a mandatory parameter for encoder like -c:a > aptx_hd? No. As opposed to: ffmpeg -i sample.wav sample.aptxhd > Perhaps we should encode everything to aptx hd by default to not have to > mess about by confusing users with these codecs or profiles things. ? It is sensible to use the aptxhd encoder by default when muxing to raw aptxhd. And it is sensible to use the aptx encoder by default when muxing to raw aptx. Or do you disagree about this ? > > Without this -profile parameter, the encoding will just fail but user > > will have to guess how to fix it. > > > > Here's how you could fix it: don't have separate muxers for aptx and > aptxhd. Make the aptx muxer handle both .aptx and .aptxhd extensions and > the codec tags. Muxer handling the codec tags ? What do you mean ? A muxer can't pass a codec tag to an encoder (encoder's init is called before the muxer's init). > > And the reported stream is much less explicit: > > Stream #0:0: Audio: aptx ([36][0][215][0] / 0xD70024), 48000 Hz, 2 > > channels, s32p > > vs. > > Stream #0:0: Audio: aptx_hd, 48000 Hz, 2 channels, s32p > > > > Profiles are still explicit > "Stream #0:0: Audio: aac (LC), 44100 Hz, stereo, fltp (16 bit), 128 kb/s" > vs > "Stream #0:0: Audio: aac (Main), 44100 Hz, stereo, fltp (16 bit), 128 kb/s" Well, this doesn't work here for aptX. I havn't checked why. Maybe codec tags is printed rather than profile, when present. > > So for the good of users and maintainers, I suggest to follow the > > wmv3/vc1 example, that is to acknowledge that aptX and aptX HD are > > commonly used as 2 separate codecs (search for > > libaptX-1.0.0-rel-Android21-ARMv7A.so and > > libaptXHD-1.0.0-rel-Android21-ARMv7A.so for example) and use the > > original patch with 2 codec IDs. > > > > Ah, android, a shining beacon of well-written, small, generic code, always > happy to reuse code and other people's work. No, wait, the other way around. Seriously ? This is just the way those two codecs are released by Qualcomm. The 2 above mentioned libs are just a common example, but they release 2 separate libs for each platform they support, so the fact that they are commonly known as 2 separate codecs is not related to Android at all. > Anyway, code-wise, there's one or two minor issues: > > +#define A2DP_VENDOR_ID_APT 0x0000004F > > +#define A2DP_APT_CODEC_ID_APTX 0x0001 > > + > > +#define A2DP_VENDOR_ID_QUALCOMM 0x000000D7 > > +#define A2DP_QUALCOMM_CODEC_ID_APTX_HD 0x0024 > > > > You have 3 copies of those in both this patch and the muxer/demuxer one. > Make libavcodec/aptx.h and put those there, then include it from lavf. Yeah, maybe. But I seem to remember that lavf was not allowed to depend on non-public headers from lavc (and I'm sure you don't want to make this header part of the public API). Am I remembering wrong ? Or has this policy changed ? > > +OBJS-$(CONFIG_APTX_HD_DECODER) += aptx.o > > +OBJS-$(CONFIG_APTX_HD_ENCODER) += aptx.o > > No point in having those flags now that there's one encoder/decoder > handling both. Oh, right. This is just a leftover. Should be removed. > > - .video_codec = AV_CODEC_ID_NONE, > > Why? Just to match the aptxhd muxer. But that should probably be in a separate patch. (it is useless to explicitely set video_codec to 0, just the same reason why we don't explicitely set subtitle_codec or data_codec) > That aside, patches look good to me. But it is also already rejected so no much point updating it for now.
On Sun, Jan 14, 2018 at 10:54:34PM +0100, Carl Eugen Hoyos wrote: > 2018-01-14 14:06 GMT+01:00 Aurelien Jacobs <aurel@gnuage.org>: > > > Well, here is an updated patch which uses codec tags for the decoder and > > profile for the encoder. > > Sorry but I object to this patch: > We should not invent codec_tags. OK, I understand, and I agree. But now we are in an interlocking situation. We have 2 solutions to handle aptX vs. aptX HD but those 2 solutions have been rejected by 2 different person. Do anybody have a 3rd solution, that everyone would accept ? And if not, how do we resolve this ? Is there any policy nowadays to handle this kind of interlocking ?
On 20 January 2018 at 17:25, Aurelien Jacobs <aurel@gnuage.org> wrote: > On Sun, Jan 14, 2018 at 05:19:12PM +0000, Rostislav Pehlivanov wrote: > > On 14 January 2018 at 13:06, Aurelien Jacobs <aurel@gnuage.org> wrote: > > > > > On Tue, Jan 09, 2018 at 02:21:02PM +0000, Rostislav Pehlivanov wrote: > > > > On 9 January 2018 at 14:07, Rostislav Pehlivanov < > atomnuker@gmail.com> > > > > wrote: > > > > > > > > > > > > > > > > > > > On 9 January 2018 at 09:00, Hendrik Leppkes <h.leppkes@gmail.com> > > > wrote: > > > > > > > > > >> On Tue, Jan 9, 2018 at 9:33 AM, Hendrik Leppkes < > h.leppkes@gmail.com> > > > > >> wrote: > > > > >> > On Tue, Jan 9, 2018 at 5:07 AM, Rostislav Pehlivanov > > > > >> > <atomnuker@gmail.com> wrote: > > > > >> >> > > > > >> >>> Anyway, all this discussion is moot as Hendrik pointed out > that > > > > >> profile > > > > >> >>> can't be set outside of lavc to determine a decoder behavior. > > > > >> >>> > > > > >> >> > > > > >> >> What, based on a comment in lavc? Comments there describe the > api > > > but > > > > >> they > > > > >> >> rarely define it. We're free to adjust them if need be and in > this > > > > >> case, > > > > >> >> since the codec profile may not be set, the API user is forced > to > > > deal > > > > >> with > > > > >> >> the lack of such set. Hence, we can clarify that it may be set > by > > > lavf > > > > >> as > > > > >> >> well as lavc as well as not being set at all. And the decoder > may > > > use > > > > >> it. > > > > >> >> I maintain that adding a new codec ID for this is unacceptable. > > > > >> >> > > > > >> > > > > > >> > We already have established methods to select codec > sub-variants, we > > > > >> > don't need to invent a new one just because you feel like it. > > > > >> > > > > > >> > > > > >> On that note, we also have cases where the same codec has > different > > > > >> codec ids based on popular known names. > > > > >> For example wmv3/vc1 - wmv3 is simple/main profile, vc1 is > advanced > > > > >> profile, different codec ids, same implementation. > > > > >> > > > > >> Re-defining public API is what is unacceptable, it requires every > > > > >> caller of lavc to suddenly start handling one more field for no > real > > > > >> reason. > > > > >> > > > > > > > > > > Then its a good thing I suggested something that doesn't involve > having > > > > > every caller of lavc to handle another field. > > > > > > > > > > > > > > > > > > > >> Either use a separate codec ID if there is sufficient reason for > that > > > > >> (mostly driven by external factors, if its handled as different > codecs > > > > >> everywhere else, not only in marketing but actual (reference) > > > > >> implementations), or use a codec_tag if one is available (the > codec id > > > > >> field from a2dp for example) > > > > > > > > > > I'd be fine with using codec tags, but not with codec IDs. > > > > > > > > Though for encoding using profiles would be best. > > > > > > Well, here is an updated patch which uses codec tags for the decoder > and > > > profile for the encoder. > > > > > > I still maintain that this solution is worse than using separate > > > codec IDs. > > > It is worse for developper / maintainer as it adds a bit of compexity > to > > > the code. > > > > > > It is worse for user as it requires adding a mandatory parameter for > > > encoding to aptX HD: > > > ffmpeg -i sample.wav -profile:a 1 sample.aptxhd > > > > > > > As opposed to having a mandatory parameter for encoder like -c:a > > aptx_hd? > > No. As opposed to: > ffmpeg -i sample.wav sample.aptxhd > > > Perhaps we should encode everything to aptx hd by default to not have to > > mess about by confusing users with these codecs or profiles things. > > ? > It is sensible to use the aptxhd encoder by default when muxing to raw > aptxhd. And it is sensible to use the aptx encoder by default when > muxing to raw aptx. > Or do you disagree about this ? > I think its sensible to specify a codec, always, and never rely on the defaults. Especially for raw formats like this. > > > > Without this -profile parameter, the encoding will just fail but user > > > will have to guess how to fix it. > > > > > > > Here's how you could fix it: don't have separate muxers for aptx and > > aptxhd. Make the aptx muxer handle both .aptx and .aptxhd extensions and > > the codec tags. > > Muxer handling the codec tags ? What do you mean ? > A muxer can't pass a codec tag to an encoder (encoder's init is called > before the muxer's init). > Not handling, accepting. You currently have 2 muxers doing the same thing but accepting different codec tags and extensions. You can make it 1 muxer accepting multiple codec tags and extensions. > > > > And the reported stream is much less explicit: > > > Stream #0:0: Audio: aptx ([36][0][215][0] / 0xD70024), 48000 Hz, 2 > > > channels, s32p > > > vs. > > > Stream #0:0: Audio: aptx_hd, 48000 Hz, 2 channels, s32p > > > > > > > Profiles are still explicit > > "Stream #0:0: Audio: aac (LC), 44100 Hz, stereo, fltp (16 bit), 128 kb/s" > > vs > > "Stream #0:0: Audio: aac (Main), 44100 Hz, stereo, fltp (16 bit), 128 > kb/s" > > Well, this doesn't work here for aptX. I havn't checked why. Maybe codec > tags is printed rather than profile, when present. > > > > So for the good of users and maintainers, I suggest to follow the > > > wmv3/vc1 example, that is to acknowledge that aptX and aptX HD are > > > commonly used as 2 separate codecs (search for > > > libaptX-1.0.0-rel-Android21-ARMv7A.so and > > > libaptXHD-1.0.0-rel-Android21-ARMv7A.so for example) and use the > > > original patch with 2 codec IDs. > > > > > > > Ah, android, a shining beacon of well-written, small, generic code, > always > > happy to reuse code and other people's work. No, wait, the other way > around. > > Seriously ? This is just the way those two codecs are released by > Qualcomm. The 2 above mentioned libs are just a common example, but they > release 2 separate libs for each platform they support, so the fact that > they are commonly known as 2 separate codecs is not related to Android > at all. > Then don't give them as an example that should be followed! That was my point and you completely missed it. > > > Anyway, code-wise, there's one or two minor issues: > > > > +#define A2DP_VENDOR_ID_APT 0x0000004F > > > +#define A2DP_APT_CODEC_ID_APTX 0x0001 > > > + > > > +#define A2DP_VENDOR_ID_QUALCOMM 0x000000D7 > > > +#define A2DP_QUALCOMM_CODEC_ID_APTX_HD 0x0024 > > > > > > > You have 3 copies of those in both this patch and the muxer/demuxer one. > > Make libavcodec/aptx.h and put those there, then include it from lavf. > > Yeah, maybe. But I seem to remember that lavf was not allowed to depend > on non-public headers from lavc (and I'm sure you don't want to make > this header part of the public API). > Am I remembering wrong ? Or has this policy changed ? > There has never been such policy. Many things in lavf depend on lavc like opus in mpegts.
On 20 January 2018 at 17:26, Aurelien Jacobs <aurel@gnuage.org> wrote: > On Sun, Jan 14, 2018 at 10:54:34PM +0100, Carl Eugen Hoyos wrote: > > 2018-01-14 14:06 GMT+01:00 Aurelien Jacobs <aurel@gnuage.org>: > > > > > Well, here is an updated patch which uses codec tags for the decoder > and > > > profile for the encoder. > > > > Sorry but I object to this patch: > > We should not invent codec_tags. > > OK, I understand, and I agree. > > But now we are in an interlocking situation. > We have 2 solutions to handle aptX vs. aptX HD but those 2 solutions have > been rejected by 2 different person. > > Do anybody have a 3rd solution, that everyone would accept ? > > And if not, how do we resolve this ? > Is there any policy nowadays to handle this kind of interlocking ? > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > Fine, I see no choice but to use multiple codec IDs for aptxhd since the format really provides you with nothing bitstream wise to determine what it is. Even game codecs have a bit or two for version.
On Sat, Jan 20, 2018 at 11:20:22PM +0000, Rostislav Pehlivanov wrote: > On 20 January 2018 at 17:26, Aurelien Jacobs <aurel@gnuage.org> wrote: > > > On Sun, Jan 14, 2018 at 10:54:34PM +0100, Carl Eugen Hoyos wrote: > > > 2018-01-14 14:06 GMT+01:00 Aurelien Jacobs <aurel@gnuage.org>: > > > > > > > Well, here is an updated patch which uses codec tags for the decoder > > and > > > > profile for the encoder. > > > > > > Sorry but I object to this patch: > > > We should not invent codec_tags. > > > > OK, I understand, and I agree. > > > > But now we are in an interlocking situation. > > We have 2 solutions to handle aptX vs. aptX HD but those 2 solutions have > > been rejected by 2 different person. > > > > Do anybody have a 3rd solution, that everyone would accept ? > > > > And if not, how do we resolve this ? > > Is there any policy nowadays to handle this kind of interlocking ? > > > > Fine, I see no choice but to use multiple codec IDs for aptxhd since the > format really provides you with nothing bitstream wise to determine what it > is. Even game codecs have a bit or two for version. OK. Good. Now, will someone push the original patchset ? Or should I rebase it and submt it again ?
On Sun, Feb 04, 2018 at 04:07:26PM +0100, Aurelien Jacobs wrote: > On Sat, Jan 20, 2018 at 11:20:22PM +0000, Rostislav Pehlivanov wrote: > > On 20 January 2018 at 17:26, Aurelien Jacobs <aurel@gnuage.org> wrote: > > > > > On Sun, Jan 14, 2018 at 10:54:34PM +0100, Carl Eugen Hoyos wrote: > > > > 2018-01-14 14:06 GMT+01:00 Aurelien Jacobs <aurel@gnuage.org>: > > > > > > > > > Well, here is an updated patch which uses codec tags for the decoder > > > and > > > > > profile for the encoder. > > > > > > > > Sorry but I object to this patch: > > > > We should not invent codec_tags. > > > > > > OK, I understand, and I agree. > > > > > > But now we are in an interlocking situation. > > > We have 2 solutions to handle aptX vs. aptX HD but those 2 solutions have > > > been rejected by 2 different person. > > > > > > Do anybody have a 3rd solution, that everyone would accept ? > > > > > > And if not, how do we resolve this ? > > > Is there any policy nowadays to handle this kind of interlocking ? > > > > > > > Fine, I see no choice but to use multiple codec IDs for aptxhd since the > > format really provides you with nothing bitstream wise to determine what it > > is. Even game codecs have a bit or two for version. > > OK. Good. > Now, will someone push the original patchset ? I think you should be able to push it yourself if there are no review comments remaining you are listed in MAINTAINERS for some parts and i see a "ajacobs" for ffmpeg so you should have git write access > Or should I rebase it and submt it again ? > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
On Mon, Feb 05, 2018 at 12:27:20AM +0100, Michael Niedermayer wrote: > On Sun, Feb 04, 2018 at 04:07:26PM +0100, Aurelien Jacobs wrote: > > On Sat, Jan 20, 2018 at 11:20:22PM +0000, Rostislav Pehlivanov wrote: > > > On 20 January 2018 at 17:26, Aurelien Jacobs <aurel@gnuage.org> wrote: > > > > > > > On Sun, Jan 14, 2018 at 10:54:34PM +0100, Carl Eugen Hoyos wrote: > > > > > 2018-01-14 14:06 GMT+01:00 Aurelien Jacobs <aurel@gnuage.org>: > > > > > > > > > > > Well, here is an updated patch which uses codec tags for the decoder > > > > and > > > > > > profile for the encoder. > > > > > > > > > > Sorry but I object to this patch: > > > > > We should not invent codec_tags. > > > > > > > > OK, I understand, and I agree. > > > > > > > > But now we are in an interlocking situation. > > > > We have 2 solutions to handle aptX vs. aptX HD but those 2 solutions have > > > > been rejected by 2 different person. > > > > > > > > Do anybody have a 3rd solution, that everyone would accept ? > > > > > > > > And if not, how do we resolve this ? > > > > Is there any policy nowadays to handle this kind of interlocking ? > > > > > > > > > > Fine, I see no choice but to use multiple codec IDs for aptxhd since the > > > format really provides you with nothing bitstream wise to determine what it > > > is. Even game codecs have a bit or two for version. > > > > OK. Good. > > > Now, will someone push the original patchset ? > > I think you should be able to push it yourself if there are no review > comments remaining > you are listed in MAINTAINERS for some parts and i see a "ajacobs" for ffmpeg > so you should have git write access OK. Well I digged out my ssh key from old laptop, an I pushed it myself.
From bdf5d5f4a3724a70efe0cad76179a57619c6fad0 Mon Sep 17 00:00:00 2001 From: Aurelien Jacobs <aurel@gnuage.org> Date: Sat, 6 Jan 2018 17:11:48 +0100 Subject: [PATCH 4/5] aptx: implement the aptX HD bluetooth codec --- Changelog | 2 +- libavcodec/Makefile | 2 + libavcodec/aptx.c | 352 +++++++++++++++++++++++++++++++++++++++++++++----- libavcodec/avcodec.h | 3 + libavcodec/profiles.c | 6 + libavcodec/profiles.h | 1 + 6 files changed, 336 insertions(+), 30 deletions(-) diff --git a/Changelog b/Changelog index 3d966c202b..9349bf1e8d 100644 --- a/Changelog +++ b/Changelog @@ -11,7 +11,7 @@ version <next>: - TiVo ty/ty+ demuxer - Intel QSV-accelerated MJPEG encoding - PCE support for extended channel layouts in the AAC encoder -- native aptX encoder and decoder +- native aptX and aptX HD encoder and decoder - Raw aptX muxer and demuxer - NVIDIA NVDEC-accelerated H.264, HEVC, MPEG-1/2/4, VC1, VP8/9 hwaccel decoding - Intel QSV-accelerated overlay filter diff --git a/libavcodec/Makefile b/libavcodec/Makefile index cfacd6b70c..a9ecf7ea5e 100644 --- a/libavcodec/Makefile +++ b/libavcodec/Makefile @@ -190,6 +190,8 @@ OBJS-$(CONFIG_ANSI_DECODER) += ansi.o cga_data.o OBJS-$(CONFIG_APE_DECODER) += apedec.o OBJS-$(CONFIG_APTX_DECODER) += aptx.o OBJS-$(CONFIG_APTX_ENCODER) += aptx.o +OBJS-$(CONFIG_APTX_HD_DECODER) += aptx.o +OBJS-$(CONFIG_APTX_HD_ENCODER) += aptx.o OBJS-$(CONFIG_APNG_DECODER) += png.o pngdec.o pngdsp.o OBJS-$(CONFIG_APNG_ENCODER) += png.o pngenc.o OBJS-$(CONFIG_SSA_DECODER) += assdec.o ass.o diff --git a/libavcodec/aptx.c b/libavcodec/aptx.c index 4173402d03..41aaa1a899 100644 --- a/libavcodec/aptx.c +++ b/libavcodec/aptx.c @@ -23,6 +23,7 @@ #include "libavutil/intreadwrite.h" #include "avcodec.h" #include "internal.h" +#include "profiles.h" #include "mathops.h" #include "audio_frame_queue.h" @@ -89,6 +90,8 @@ typedef struct { } Channel; typedef struct { + int hd; + int block_size; int32_t sync_idx; Channel channels[NB_CHANNELS]; AudioFrameQueue afq; @@ -182,6 +185,205 @@ static const int16_t quantize_factor_select_offset_HF[5] = { 0, -8, 33, 95, 262, }; + +static const int32_t hd_quantize_intervals_LF[257] = { + -2436, 2436, 7308, 12180, 17054, 21930, 26806, 31686, + 36566, 41450, 46338, 51230, 56124, 61024, 65928, 70836, + 75750, 80670, 85598, 90530, 95470, 100418, 105372, 110336, + 115308, 120288, 125278, 130276, 135286, 140304, 145334, 150374, + 155426, 160490, 165566, 170654, 175756, 180870, 185998, 191138, + 196294, 201466, 206650, 211850, 217068, 222300, 227548, 232814, + 238096, 243396, 248714, 254050, 259406, 264778, 270172, 275584, + 281018, 286470, 291944, 297440, 302956, 308496, 314056, 319640, + 325248, 330878, 336532, 342212, 347916, 353644, 359398, 365178, + 370986, 376820, 382680, 388568, 394486, 400430, 406404, 412408, + 418442, 424506, 430600, 436726, 442884, 449074, 455298, 461554, + 467844, 474168, 480528, 486922, 493354, 499820, 506324, 512866, + 519446, 526064, 532722, 539420, 546160, 552940, 559760, 566624, + 573532, 580482, 587478, 594520, 601606, 608740, 615920, 623148, + 630426, 637754, 645132, 652560, 660042, 667576, 675164, 682808, + 690506, 698262, 706074, 713946, 721876, 729868, 737920, 746036, + 754216, 762460, 770770, 779148, 787594, 796108, 804694, 813354, + 822086, 830892, 839774, 848736, 857776, 866896, 876100, 885386, + 894758, 904218, 913766, 923406, 933138, 942964, 952886, 962908, + 973030, 983254, 993582, 1004020, 1014566, 1025224, 1035996, 1046886, + 1057894, 1069026, 1080284, 1091670, 1103186, 1114838, 1126628, 1138558, + 1150634, 1162858, 1175236, 1187768, 1200462, 1213320, 1226346, 1239548, + 1252928, 1266490, 1280242, 1294188, 1308334, 1322688, 1337252, 1352034, + 1367044, 1382284, 1397766, 1413494, 1429478, 1445728, 1462252, 1479058, + 1496158, 1513562, 1531280, 1549326, 1567710, 1586446, 1605550, 1625034, + 1644914, 1665208, 1685932, 1707108, 1728754, 1750890, 1773542, 1796732, + 1820488, 1844840, 1869816, 1895452, 1921780, 1948842, 1976680, 2005338, + 2034868, 2065322, 2096766, 2129260, 2162880, 2197708, 2233832, 2271352, + 2310384, 2351050, 2393498, 2437886, 2484404, 2533262, 2584710, 2639036, + 2696578, 2757738, 2822998, 2892940, 2968278, 3049896, 3138912, 3236760, + 3345312, 3467068, 3605434, 3765154, 3952904, 4177962, 4452178, 4787134, + 5187290, 5647128, 6159120, 6720518, 7332904, 8000032, 8726664, 9518152, + 10380372, +}; +static const int32_t hd_invert_quantize_dither_factors_LF[257] = { + 2436, 2436, 2436, 2436, 2438, 2438, 2438, 2440, + 2442, 2442, 2444, 2446, 2448, 2450, 2454, 2456, + 2458, 2462, 2464, 2468, 2472, 2476, 2480, 2484, + 2488, 2492, 2498, 2502, 2506, 2512, 2518, 2524, + 2528, 2534, 2540, 2548, 2554, 2560, 2568, 2574, + 2582, 2588, 2596, 2604, 2612, 2620, 2628, 2636, + 2646, 2654, 2664, 2672, 2682, 2692, 2702, 2712, + 2722, 2732, 2742, 2752, 2764, 2774, 2786, 2798, + 2810, 2822, 2834, 2846, 2858, 2870, 2884, 2896, + 2910, 2924, 2938, 2952, 2966, 2980, 2994, 3010, + 3024, 3040, 3056, 3070, 3086, 3104, 3120, 3136, + 3154, 3170, 3188, 3206, 3224, 3242, 3262, 3280, + 3300, 3320, 3338, 3360, 3380, 3400, 3422, 3442, + 3464, 3486, 3508, 3532, 3554, 3578, 3602, 3626, + 3652, 3676, 3702, 3728, 3754, 3780, 3808, 3836, + 3864, 3892, 3920, 3950, 3980, 4010, 4042, 4074, + 4106, 4138, 4172, 4206, 4240, 4276, 4312, 4348, + 4384, 4422, 4460, 4500, 4540, 4580, 4622, 4664, + 4708, 4752, 4796, 4842, 4890, 4938, 4986, 5036, + 5086, 5138, 5192, 5246, 5300, 5358, 5416, 5474, + 5534, 5596, 5660, 5726, 5792, 5860, 5930, 6002, + 6074, 6150, 6226, 6306, 6388, 6470, 6556, 6644, + 6736, 6828, 6924, 7022, 7124, 7228, 7336, 7448, + 7562, 7680, 7802, 7928, 8058, 8192, 8332, 8476, + 8624, 8780, 8940, 9106, 9278, 9458, 9644, 9840, + 10042, 10252, 10472, 10702, 10942, 11194, 11458, 11734, + 12024, 12328, 12648, 12986, 13342, 13720, 14118, 14540, + 14990, 15466, 15976, 16520, 17102, 17726, 18398, 19124, + 19908, 20760, 21688, 22702, 23816, 25044, 26404, 27922, + 29622, 31540, 33720, 36222, 39116, 42502, 46514, 51334, + 57218, 64536, 73830, 85890, 101860, 123198, 151020, 183936, + 216220, 243618, 268374, 293022, 319362, 347768, 378864, 412626, 449596, +}; +static const int32_t hd_quantize_dither_factors_LF[256] = { + 0, 0, 0, 1, 0, 0, 1, 1, + 0, 1, 1, 1, 1, 1, 1, 1, + 1, 1, 1, 1, 1, 1, 1, 1, + 1, 2, 1, 1, 2, 2, 2, 1, + 2, 2, 2, 2, 2, 2, 2, 2, + 2, 2, 2, 2, 2, 2, 2, 3, + 2, 3, 2, 3, 3, 3, 3, 3, + 3, 3, 3, 3, 3, 3, 3, 3, + 3, 3, 3, 3, 3, 4, 3, 4, + 4, 4, 4, 4, 4, 4, 4, 4, + 4, 4, 4, 4, 5, 4, 4, 5, + 4, 5, 5, 5, 5, 5, 5, 5, + 5, 5, 6, 5, 5, 6, 5, 6, + 6, 6, 6, 6, 6, 6, 6, 7, + 6, 7, 7, 7, 7, 7, 7, 7, + 7, 7, 8, 8, 8, 8, 8, 8, + 8, 9, 9, 9, 9, 9, 9, 9, + 10, 10, 10, 10, 10, 11, 11, 11, + 11, 11, 12, 12, 12, 12, 13, 13, + 13, 14, 14, 14, 15, 15, 15, 15, + 16, 16, 17, 17, 17, 18, 18, 18, + 19, 19, 20, 21, 21, 22, 22, 23, + 23, 24, 25, 26, 26, 27, 28, 29, + 30, 31, 32, 33, 34, 35, 36, 37, + 39, 40, 42, 43, 45, 47, 49, 51, + 53, 55, 58, 60, 63, 66, 69, 73, + 76, 80, 85, 89, 95, 100, 106, 113, + 119, 128, 136, 146, 156, 168, 182, 196, + 213, 232, 254, 279, 307, 340, 380, 425, + 480, 545, 626, 724, 847, 1003, 1205, 1471, + 1830, 2324, 3015, 3993, 5335, 6956, 8229, 8071, + 6850, 6189, 6162, 6585, 7102, 7774, 8441, 9243, +}; +static const int16_t hd_quantize_factor_select_offset_LF[257] = { + 0, -22, -21, -21, -20, -20, -19, -19, + -18, -18, -17, -17, -16, -16, -15, -14, + -14, -13, -13, -12, -12, -11, -11, -10, + -10, -9, -9, -8, -7, -7, -6, -6, + -5, -5, -4, -4, -3, -3, -2, -1, + -1, 0, 0, 1, 1, 2, 2, 3, + 4, 4, 5, 5, 6, 6, 7, 8, + 8, 9, 9, 10, 11, 11, 12, 12, + 13, 14, 14, 15, 15, 16, 17, 17, + 18, 19, 19, 20, 20, 21, 22, 22, + 23, 24, 24, 25, 26, 26, 27, 28, + 28, 29, 30, 30, 31, 32, 33, 33, + 34, 35, 35, 36, 37, 38, 38, 39, + 40, 41, 41, 42, 43, 44, 44, 45, + 46, 47, 48, 48, 49, 50, 51, 52, + 52, 53, 54, 55, 56, 57, 58, 58, + 59, 60, 61, 62, 63, 64, 65, 66, + 67, 68, 69, 69, 70, 71, 72, 73, + 74, 75, 77, 78, 79, 80, 81, 82, + 83, 84, 85, 86, 87, 89, 90, 91, + 92, 93, 94, 96, 97, 98, 99, 101, + 102, 103, 105, 106, 107, 109, 110, 112, + 113, 115, 116, 118, 119, 121, 122, 124, + 125, 127, 129, 130, 132, 134, 136, 137, + 139, 141, 143, 145, 147, 149, 151, 153, + 155, 158, 160, 162, 164, 167, 169, 172, + 174, 177, 180, 182, 185, 188, 191, 194, + 197, 201, 204, 208, 211, 215, 219, 223, + 227, 232, 236, 241, 246, 251, 257, 263, + 269, 275, 283, 290, 298, 307, 317, 327, + 339, 352, 367, 384, 404, 429, 458, 494, + 522, 522, 522, 522, 522, 522, 522, 522, 522, +}; + + +static const int32_t hd_quantize_intervals_MLF[33] = { + -21236, 21236, 63830, 106798, 150386, 194832, 240376, 287258, + 335726, 386034, 438460, 493308, 550924, 611696, 676082, 744626, + 817986, 896968, 982580, 1076118, 1179278, 1294344, 1424504, 1574386, + 1751090, 1966260, 2240868, 2617662, 3196432, 4176450, 5658260, 7671068, + 10380372, +}; +static const int32_t hd_invert_quantize_dither_factors_MLF[33] = { + 21236, 21236, 21360, 21608, 21978, 22468, 23076, 23806, + 24660, 25648, 26778, 28070, 29544, 31228, 33158, 35386, + 37974, 41008, 44606, 48934, 54226, 60840, 69320, 80564, + 96140, 119032, 155576, 221218, 357552, 622468, 859344, 1153464, 1555840, +}; +static const int32_t hd_quantize_dither_factors_MLF[32] = { + 0, 31, 62, 93, 123, 152, 183, 214, + 247, 283, 323, 369, 421, 483, 557, 647, + 759, 900, 1082, 1323, 1654, 2120, 2811, 3894, + 5723, 9136, 16411, 34084, 66229, 59219, 73530, 100594, +}; +static const int16_t hd_quantize_factor_select_offset_MLF[33] = { + 0, -21, -16, -12, -7, -2, 3, 8, + 13, 19, 24, 30, 36, 43, 50, 57, + 65, 74, 83, 93, 104, 117, 131, 147, + 166, 189, 219, 259, 322, 427, 521, 521, 521, +}; + + +static const int32_t hd_quantize_intervals_MHF[9] = { + -95044, 95044, 295844, 528780, 821332, 1226438, 1890540, 3344850, 6450664, +}; +static const int32_t hd_invert_quantize_dither_factors_MHF[9] = { + 95044, 95044, 105754, 127180, 165372, 39736, 424366, 1029946, 2075866, +}; +static const int32_t hd_quantize_dither_factors_MHF[8] = { + 0, 2678, 5357, 9548, -31409, 96158, 151395, 261480, +}; +static const int16_t hd_quantize_factor_select_offset_MHF[9] = { + 0, -17, 5, 30, 62, 105, 177, 334, 518, +}; + + +static const int32_t hd_quantize_intervals_HF[17] = { + -45754, 45754, 138496, 234896, 337336, 448310, 570738, 708380, + 866534, 1053262, 1281958, 1577438, 1993050, 2665984, 3900982, 5902844, + 8897462, +}; +static const int32_t hd_invert_quantize_dither_factors_HF[17] = { + 45754, 45754, 46988, 49412, 53026, 57950, 64478, 73164, + 84988, 101740, 126958, 168522, 247092, 425842, 809154, 1192708, 1801910, +}; +static const int32_t hd_quantize_dither_factors_HF[16] = { + 0, 309, 606, 904, 1231, 1632, 2172, 2956, + 4188, 6305, 10391, 19643, 44688, 95828, 95889, 152301, +}; +static const int16_t hd_quantize_factor_select_offset_HF[17] = { + 0, -18, -8, 2, 13, 25, 38, 53, + 70, 90, 115, 147, 192, 264, 398, 521, 521, +}; + typedef const struct { const int32_t *quantize_intervals; const int32_t *invert_quantize_dither_factors; @@ -192,7 +394,8 @@ typedef const struct { int32_t prediction_order; } ConstTables; -static ConstTables tables[NB_SUBBANDS] = { +static ConstTables tables[2][NB_SUBBANDS] = { +{ [LF] = { quantize_intervals_LF, invert_quantize_dither_factors_LF, quantize_dither_factors_LF, @@ -217,6 +420,33 @@ static ConstTables tables[NB_SUBBANDS] = { quantize_factor_select_offset_HF, FF_ARRAY_ELEMS(quantize_intervals_HF), 0x15FF, 12 }, +}, +{ + [LF] = { hd_quantize_intervals_LF, + hd_invert_quantize_dither_factors_LF, + hd_quantize_dither_factors_LF, + hd_quantize_factor_select_offset_LF, + FF_ARRAY_ELEMS(hd_quantize_intervals_LF), + 0x11FF, 24 }, + [MLF] = { hd_quantize_intervals_MLF, + hd_invert_quantize_dither_factors_MLF, + hd_quantize_dither_factors_MLF, + hd_quantize_factor_select_offset_MLF, + FF_ARRAY_ELEMS(hd_quantize_intervals_MLF), + 0x14FF, 12 }, + [MHF] = { hd_quantize_intervals_MHF, + hd_invert_quantize_dither_factors_MHF, + hd_quantize_dither_factors_MHF, + hd_quantize_factor_select_offset_MHF, + FF_ARRAY_ELEMS(hd_quantize_intervals_MHF), + 0x16FF, 6 }, + [HF] = { hd_quantize_intervals_HF, + hd_invert_quantize_dither_factors_HF, + hd_quantize_dither_factors_HF, + hd_quantize_factor_select_offset_HF, + FF_ARRAY_ELEMS(hd_quantize_intervals_HF), + 0x15FF, 12 }, +} }; static const int16_t quantization_factors[32] = { @@ -494,7 +724,7 @@ static void aptx_quantize_difference(Quantize *quantize, quantize->quantized_sample_parity_change = parity_change ^ inv; } -static void aptx_encode_channel(Channel *channel, int32_t samples[4]) +static void aptx_encode_channel(Channel *channel, int32_t samples[4], int hd) { int32_t subband_samples[4]; int subband; @@ -505,7 +735,7 @@ static void aptx_encode_channel(Channel *channel, int32_t samples[4]) aptx_quantize_difference(&channel->quantize[subband], diff, channel->dither[subband], channel->invert_quantize[subband].quantization_factor, - &tables[subband]); + &tables[hd][subband]); } } @@ -616,7 +846,7 @@ static void aptx_process_subband(InvertQuantize *invert_quantize, tables->prediction_order); } -static void aptx_invert_quantize_and_prediction(Channel *channel) +static void aptx_invert_quantize_and_prediction(Channel *channel, int hd) { int subband; for (subband = 0; subband < NB_SUBBANDS; subband++) @@ -624,7 +854,7 @@ static void aptx_invert_quantize_and_prediction(Channel *channel) &channel->prediction[subband], channel->quantize[subband].quantized_sample, channel->dither[subband], - &tables[subband]); + &tables[hd][subband]); } static int32_t aptx_quantized_parity(Channel *channel) @@ -678,6 +908,15 @@ static uint16_t aptx_pack_codeword(Channel *channel) | (((channel->quantize[0].quantized_sample & 0x7F) ) << 0); } +static uint32_t aptxhd_pack_codeword(Channel *channel) +{ + int32_t parity = aptx_quantized_parity(channel); + return (((channel->quantize[3].quantized_sample & 0x01E) | parity) << 19) + | (((channel->quantize[2].quantized_sample & 0x00F) ) << 15) + | (((channel->quantize[1].quantized_sample & 0x03F) ) << 9) + | (((channel->quantize[0].quantized_sample & 0x1FF) ) << 0); +} + static void aptx_unpack_codeword(Channel *channel, uint16_t codeword) { channel->quantize[0].quantized_sample = sign_extend(codeword >> 0, 7); @@ -688,35 +927,53 @@ static void aptx_unpack_codeword(Channel *channel, uint16_t codeword) | aptx_quantized_parity(channel); } +static void aptxhd_unpack_codeword(Channel *channel, uint32_t codeword) +{ + channel->quantize[0].quantized_sample = sign_extend(codeword >> 0, 9); + channel->quantize[1].quantized_sample = sign_extend(codeword >> 9, 6); + channel->quantize[2].quantized_sample = sign_extend(codeword >> 15, 4); + channel->quantize[3].quantized_sample = sign_extend(codeword >> 19, 5); + channel->quantize[3].quantized_sample = (channel->quantize[3].quantized_sample & ~1) + | aptx_quantized_parity(channel); +} + static void aptx_encode_samples(AptXContext *ctx, int32_t samples[NB_CHANNELS][4], - uint8_t output[2*NB_CHANNELS]) + uint8_t *output) { int channel; for (channel = 0; channel < NB_CHANNELS; channel++) - aptx_encode_channel(&ctx->channels[channel], samples[channel]); + aptx_encode_channel(&ctx->channels[channel], samples[channel], ctx->hd); aptx_insert_sync(ctx->channels, &ctx->sync_idx); for (channel = 0; channel < NB_CHANNELS; channel++) { - aptx_invert_quantize_and_prediction(&ctx->channels[channel]); - AV_WB16(output + 2*channel, aptx_pack_codeword(&ctx->channels[channel])); + aptx_invert_quantize_and_prediction(&ctx->channels[channel], ctx->hd); + if (ctx->hd) + AV_WB24(output + 3*channel, + aptxhd_pack_codeword(&ctx->channels[channel])); + else + AV_WB16(output + 2*channel, + aptx_pack_codeword(&ctx->channels[channel])); } } static int aptx_decode_samples(AptXContext *ctx, - const uint8_t input[2*NB_CHANNELS], + const uint8_t *input, int32_t samples[NB_CHANNELS][4]) { int channel, ret; for (channel = 0; channel < NB_CHANNELS; channel++) { - uint16_t codeword; aptx_generate_dither(&ctx->channels[channel]); - codeword = AV_RB16(input + 2*channel); - aptx_unpack_codeword(&ctx->channels[channel], codeword); - aptx_invert_quantize_and_prediction(&ctx->channels[channel]); + if (ctx->hd) + aptxhd_unpack_codeword(&ctx->channels[channel], + AV_RB24(input + 3*channel)); + else + aptx_unpack_codeword(&ctx->channels[channel], + AV_RB16(input + 2*channel)); + aptx_invert_quantize_and_prediction(&ctx->channels[channel], ctx->hd); } ret = aptx_check_parity(ctx->channels, &ctx->sync_idx); @@ -733,11 +990,14 @@ static av_cold int aptx_init(AVCodecContext *avctx) AptXContext *s = avctx->priv_data; int chan, subband; + s->block_size = s->hd ? 6 : 4; + if (avctx->frame_size == 0) - avctx->frame_size = 1024; + avctx->frame_size = 256 * s->block_size; - if (avctx->frame_size & 3) { - av_log(avctx, AV_LOG_ERROR, "Frame size must be a multiple of 4 samples\n"); + if (avctx->frame_size % s->block_size) { + av_log(avctx, AV_LOG_ERROR, + "Frame size must be a multiple of %d samples\n", s->block_size); return AVERROR(EINVAL); } @@ -754,14 +1014,46 @@ static av_cold int aptx_init(AVCodecContext *avctx) return 0; } +#define A2DP_VENDOR_ID_APT 0x0000004F +#define A2DP_APT_CODEC_ID_APTX 0x0001 + +#define A2DP_VENDOR_ID_QUALCOMM 0x000000D7 +#define A2DP_QUALCOMM_CODEC_ID_APTX_HD 0x0024 + +static av_cold int aptx_decoder_init(AVCodecContext *avctx) +{ + AptXContext *s = avctx->priv_data; + + s->hd = avctx->codec_tag == ((A2DP_VENDOR_ID_QUALCOMM << 16) + | A2DP_QUALCOMM_CODEC_ID_APTX_HD); + avctx->profile = s->hd ? FF_PROFILE_APTX_HD : FF_PROFILE_APTX; + + return aptx_init(avctx); +} + +static av_cold int aptx_encoder_init(AVCodecContext *avctx) +{ + AptXContext *s = avctx->priv_data; + + s->hd = avctx->profile == FF_PROFILE_APTX_HD; + if (s->hd) + avctx->codec_tag = (A2DP_VENDOR_ID_QUALCOMM << 16) + | A2DP_QUALCOMM_CODEC_ID_APTX_HD; + else + avctx->codec_tag = (A2DP_VENDOR_ID_APT << 16) + | A2DP_APT_CODEC_ID_APTX; + + return aptx_init(avctx); +} + static int aptx_decode_frame(AVCodecContext *avctx, void *data, int *got_frame_ptr, AVPacket *avpkt) { AptXContext *s = avctx->priv_data; AVFrame *frame = data; - int pos, channel, sample, ret; + int pos, opos, channel, sample, ret; - if (avpkt->size < 4) { + if (avpkt->size < s->block_size) { av_log(avctx, AV_LOG_ERROR, "Packet is too small\n"); return AVERROR_INVALIDDATA; } @@ -769,11 +1061,11 @@ static int aptx_decode_frame(AVCodecContext *avctx, void *data, /* get output buffer */ frame->channels = NB_CHANNELS; frame->format = AV_SAMPLE_FMT_S32P; - frame->nb_samples = avpkt->size & ~3; + frame->nb_samples = 4 * avpkt->size / s->block_size; if ((ret = ff_get_buffer(avctx, frame, 0)) < 0) return ret; - for (pos = 0; pos < frame->nb_samples; pos += 4) { + for (pos = 0, opos = 0; opos < frame->nb_samples; pos += s->block_size, opos += 4) { int32_t samples[NB_CHANNELS][4]; if (aptx_decode_samples(s, &avpkt->data[pos], samples)) { @@ -783,32 +1075,33 @@ static int aptx_decode_frame(AVCodecContext *avctx, void *data, for (channel = 0; channel < NB_CHANNELS; channel++) for (sample = 0; sample < 4; sample++) - AV_WN32A(&frame->data[channel][4*(sample+pos)], + AV_WN32A(&frame->data[channel][4*(opos+sample)], samples[channel][sample] << 8); } *got_frame_ptr = 1; - return frame->nb_samples; + return s->block_size * frame->nb_samples / 4; } static int aptx_encode_frame(AVCodecContext *avctx, AVPacket *avpkt, const AVFrame *frame, int *got_packet_ptr) { AptXContext *s = avctx->priv_data; - int pos, channel, sample, ret; + int pos, ipos, channel, sample, output_size, ret; if ((ret = ff_af_queue_add(&s->afq, frame)) < 0) return ret; - if ((ret = ff_alloc_packet2(avctx, avpkt, frame->nb_samples, 0)) < 0) + output_size = s->block_size * frame->nb_samples/4; + if ((ret = ff_alloc_packet2(avctx, avpkt, output_size, 0)) < 0) return ret; - for (pos = 0; pos < frame->nb_samples; pos += 4) { + for (pos = 0, ipos = 0; pos < output_size; pos += s->block_size, ipos += 4) { int32_t samples[NB_CHANNELS][4]; for (channel = 0; channel < NB_CHANNELS; channel++) for (sample = 0; sample < 4; sample++) - samples[channel][sample] = (int32_t)AV_RN32A(&frame->data[channel][4*(sample+pos)]) >> 8; + samples[channel][sample] = (int32_t)AV_RN32A(&frame->data[channel][4*(ipos+sample)]) >> 8; aptx_encode_samples(s, samples, avpkt->data + pos); } @@ -833,7 +1126,7 @@ AVCodec ff_aptx_decoder = { .type = AVMEDIA_TYPE_AUDIO, .id = AV_CODEC_ID_APTX, .priv_data_size = sizeof(AptXContext), - .init = aptx_init, + .init = aptx_decoder_init, .decode = aptx_decode_frame, .close = aptx_close, .capabilities = AV_CODEC_CAP_DR1, @@ -851,7 +1144,7 @@ AVCodec ff_aptx_encoder = { .type = AVMEDIA_TYPE_AUDIO, .id = AV_CODEC_ID_APTX, .priv_data_size = sizeof(AptXContext), - .init = aptx_init, + .init = aptx_encoder_init, .encode2 = aptx_encode_frame, .close = aptx_close, .capabilities = AV_CODEC_CAP_SMALL_LAST_FRAME, @@ -860,5 +1153,6 @@ AVCodec ff_aptx_encoder = { .sample_fmts = (const enum AVSampleFormat[]) { AV_SAMPLE_FMT_S32P, AV_SAMPLE_FMT_NONE }, .supported_samplerates = (const int[]) {8000, 16000, 24000, 32000, 44100, 48000, 0}, + .profiles = NULL_IF_CONFIG_SMALL(ff_aptx_profiles), }; #endif diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h index c13deb599f..d6dc9bd5b5 100644 --- a/libavcodec/avcodec.h +++ b/libavcodec/avcodec.h @@ -2839,6 +2839,9 @@ typedef struct AVCodecContext { #define FF_PROFILE_MPEG2_AAC_LOW 128 #define FF_PROFILE_MPEG2_AAC_HE 131 +#define FF_PROFILE_APTX 0 +#define FF_PROFILE_APTX_HD 1 + #define FF_PROFILE_DNXHD 0 #define FF_PROFILE_DNXHR_LB 1 #define FF_PROFILE_DNXHR_SQ 2 diff --git a/libavcodec/profiles.c b/libavcodec/profiles.c index 30498efedf..5db8839cc7 100644 --- a/libavcodec/profiles.c +++ b/libavcodec/profiles.c @@ -36,6 +36,12 @@ const AVProfile ff_aac_profiles[] = { { FF_PROFILE_UNKNOWN }, }; +const AVProfile ff_aptx_profiles[] = { + { FF_PROFILE_APTX, "aptX" }, + { FF_PROFILE_APTX_HD, "aptX HD" }, + { FF_PROFILE_UNKNOWN }, +}; + const AVProfile ff_dca_profiles[] = { { FF_PROFILE_DTS, "DTS" }, { FF_PROFILE_DTS_ES, "DTS-ES" }, diff --git a/libavcodec/profiles.h b/libavcodec/profiles.h index eb18b406af..c01174cd61 100644 --- a/libavcodec/profiles.h +++ b/libavcodec/profiles.h @@ -22,6 +22,7 @@ #include "avcodec.h" extern const AVProfile ff_aac_profiles[]; +extern const AVProfile ff_aptx_profiles[]; extern const AVProfile ff_dca_profiles[]; extern const AVProfile ff_dnxhd_profiles[]; extern const AVProfile ff_h264_profiles[]; -- 2.15.1