[FFmpeg-devel] avcodec/als: use planar sample formats

Submitted by Paul B Mahol on July 1, 2017, 8:23 p.m.

Details

Message ID 20170701202307.2243-1-onemda@gmail.com
State New
Headers show

Commit Message

Paul B Mahol July 1, 2017, 8:23 p.m.
This is native layout of this codec.

Signed-off-by: Paul B Mahol <onemda@gmail.com>
---
 libavcodec/alsdec.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

Comments

Thilo Borgmann July 2, 2017, 2:12 p.m.
Am 01.07.17 um 22:23 schrieb Paul B Mahol:
> This is native layout of this codec.

From where is that definition?

-Thilo
Paul B Mahol July 2, 2017, 2:17 p.m.
On 7/2/17, Thilo Borgmann <thilo.borgmann@mail.de> wrote:
> Am 01.07.17 um 22:23 schrieb Paul B Mahol:
>> This is native layout of this codec.
>
> From where is that definition?

See how samples are stored in raw buffers.

Many codecs do not do interleaving and that work is left to another library.
Thilo Borgmann July 4, 2017, 7:34 p.m.
Am 02.07.17 um 16:17 schrieb Paul B Mahol:
> On 7/2/17, Thilo Borgmann <thilo.borgmann@mail.de> wrote:
>> Am 01.07.17 um 22:23 schrieb Paul B Mahol:
>>> This is native layout of this codec.
>>
>> From where is that definition?
> 
> See how samples are stored in raw buffers.

The raw buffers are our own construct and as such hardly an argument for that.
If you mean that in the bitstream the most inner loop iterates over the samples
of a given block of a given channel then, well, one might argue that planer is
native...


> Many codecs do not do interleaving and that work is left to another library.

Sound like a good way to handle it.

However, I think we can let the user decide what output format he likes, don't
we? So I am ok with adding planer output format next to interleaved - I do not
follow you on just replacing it (assuming the former is true). I'd also be ok
with defining planer output to be the default in that case.

-Thilo
Paul B Mahol July 4, 2017, 7:35 p.m.
On 7/4/17, Thilo Borgmann <thilo.borgmann@mail.de> wrote:
> Am 02.07.17 um 16:17 schrieb Paul B Mahol:
>> On 7/2/17, Thilo Borgmann <thilo.borgmann@mail.de> wrote:
>>> Am 01.07.17 um 22:23 schrieb Paul B Mahol:
>>>> This is native layout of this codec.
>>>
>>> From where is that definition?
>>
>> See how samples are stored in raw buffers.
>
> The raw buffers are our own construct and as such hardly an argument for
> that.
> If you mean that in the bitstream the most inner loop iterates over the
> samples
> of a given block of a given channel then, well, one might argue that planer
> is
> native...
>
>
>> Many codecs do not do interleaving and that work is left to another
>> library.
>
> Sound like a good way to handle it.
>
> However, I think we can let the user decide what output format he likes,
> don't
> we? So I am ok with adding planer output format next to interleaved - I do
> not
> follow you on just replacing it (assuming the former is true). I'd also be
> ok
> with defining planer output to be the default in that case.
>
> -Thilo
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

I already dropped the patch.
wm4 July 5, 2017, 7:46 a.m.
On Tue, 4 Jul 2017 21:34:26 +0200
Thilo Borgmann <thilo.borgmann@mail.de> wrote:

> Am 02.07.17 um 16:17 schrieb Paul B Mahol:
> > On 7/2/17, Thilo Borgmann <thilo.borgmann@mail.de> wrote:  
> >> Am 01.07.17 um 22:23 schrieb Paul B Mahol:  
> >>> This is native layout of this codec.  
> >>
> >> From where is that definition?  
> > 
> > See how samples are stored in raw buffers.  
> 
> The raw buffers are our own construct and as such hardly an argument for that.
> If you mean that in the bitstream the most inner loop iterates over the samples
> of a given block of a given channel then, well, one might argue that planer is
> native...
> 
> 
> > Many codecs do not do interleaving and that work is left to another library.  
> 
> Sound like a good way to handle it.
> 
> However, I think we can let the user decide what output format he likes, don't
> we? So I am ok with adding planer output format next to interleaved - I do not
> follow you on just replacing it (assuming the former is true). I'd also be ok
> with defining planer output to be the default in that case.

We've changed the output format of decoders before. The user can't
expect a specific format.

Patch hide | download patch | download mbox

diff --git a/libavcodec/alsdec.c b/libavcodec/alsdec.c
index 4a8f13d..31e95e2 100644
--- a/libavcodec/alsdec.c
+++ b/libavcodec/alsdec.c
@@ -1790,25 +1790,28 @@  static int decode_frame(AVCodecContext *avctx, void *data, int *got_frame_ptr,
         return ret;
 
     // transform decoded frame into output format
-    #define INTERLEAVE_OUTPUT(bps)                                                   \
+    #define SHIFT_OUTPUT(bps)                                                        \
     {                                                                                \
-        int##bps##_t *dest = (int##bps##_t*)frame->data[0];                          \
         shift = bps - ctx->avctx->bits_per_raw_sample;                               \
         if (!ctx->cs_switch) {                                                       \
-            for (sample = 0; sample < ctx->cur_frame_length; sample++)               \
-                for (c = 0; c < avctx->channels; c++)                                \
+            for (c = 0; c < avctx->channels; c++) {                                  \
+                int##bps##_t *dest = (int##bps##_t*)frame->extended_data[c];         \
+                for (sample = 0; sample < ctx->cur_frame_length; sample++)           \
                     *dest++ = ctx->raw_samples[c][sample] << shift;                  \
+            }                                                                        \
         } else {                                                                     \
-            for (sample = 0; sample < ctx->cur_frame_length; sample++)               \
-                for (c = 0; c < avctx->channels; c++)                                \
+            for (c = 0; c < avctx->channels; c++) {                                  \
+                int##bps##_t *dest = (int##bps##_t*)frame->extended_data[c];         \
+                for (sample = 0; sample < ctx->cur_frame_length; sample++)           \
                     *dest++ = ctx->raw_samples[sconf->chan_pos[c]][sample] << shift; \
+            }                                                                        \
         }                                                                            \
     }
 
     if (ctx->avctx->bits_per_raw_sample <= 16) {
-        INTERLEAVE_OUTPUT(16)
+        SHIFT_OUTPUT(16)
     } else {
-        INTERLEAVE_OUTPUT(32)
+        SHIFT_OUTPUT(32)
     }
 
     // update CRC
@@ -1960,11 +1963,11 @@  static av_cold int decode_init(AVCodecContext *avctx)
             goto fail;
     }
     if (sconf->floating) {
-        avctx->sample_fmt          = AV_SAMPLE_FMT_FLT;
+        avctx->sample_fmt          = AV_SAMPLE_FMT_FLTP;
         avctx->bits_per_raw_sample = 32;
     } else {
         avctx->sample_fmt          = sconf->resolution > 1
-                                     ? AV_SAMPLE_FMT_S32 : AV_SAMPLE_FMT_S16;
+                                     ? AV_SAMPLE_FMT_S32P : AV_SAMPLE_FMT_S16P;
         avctx->bits_per_raw_sample = (sconf->resolution + 1) * 8;
         if (avctx->bits_per_raw_sample > 32) {
             av_log(avctx, AV_LOG_ERROR, "Bits per raw sample %d larger than 32.\n",