[01/10] ac3enc: allow new coupling coordinates to be sent independently for each channel.

Message ID 1312759748-2752-2-git-send-email-justin.ruggles@gmail.com
State Committed
Commit c3d63262fe61a8e219c4b69c61af6e0c9d49be2d
Headers show

Commit Message

Justin Ruggles Aug. 7, 2011, 11:28 p.m.
---
 libavcodec/ac3enc.c          |   10 ++++----
 libavcodec/ac3enc.h          |    2 +-
 libavcodec/ac3enc_template.c |   50 ++++++++++++++++--------------------------
 libavcodec/eac3enc.c         |    2 +-
 4 files changed, 26 insertions(+), 38 deletions(-)

Comments

Ronald Bultje Aug. 8, 2011, 11:18 p.m. | #1
Hi,

On Sun, Aug 7, 2011 at 4:28 PM, Justin Ruggles <justin.ruggles@gmail.com> wrote:
> ---
>  libavcodec/ac3enc.c          |   10 ++++----
>  libavcodec/ac3enc.h          |    2 +-
>  libavcodec/ac3enc_template.c |   50 ++++++++++++++++--------------------------
>  libavcodec/eac3enc.c         |    2 +-
>  4 files changed, 26 insertions(+), 38 deletions(-)

In general, looks OK.

Something that'd help me (as a future patch, doesn't need to be part
of this one right away since issue already exists) is if you could
document what the values 0, 1 and 2 for new_cpl_coords[] means. 0
means "no channel coupling", 1 means "ac3 uses channel coupling", and
2 has something to do with eac3 (default channel coordinates or so?),
but I'm not quite sure what.

Ronald
Justin Ruggles Aug. 9, 2011, 10:55 a.m. | #2
On 08/08/2011 07:18 PM, Ronald S. Bultje wrote:

> Hi,
> 
> On Sun, Aug 7, 2011 at 4:28 PM, Justin Ruggles <justin.ruggles@gmail.com> wrote:
>> ---
>>  libavcodec/ac3enc.c          |   10 ++++----
>>  libavcodec/ac3enc.h          |    2 +-
>>  libavcodec/ac3enc_template.c |   50 ++++++++++++++++--------------------------
>>  libavcodec/eac3enc.c         |    2 +-
>>  4 files changed, 26 insertions(+), 38 deletions(-)
> 
> In general, looks OK.
> 
> Something that'd help me (as a future patch, doesn't need to be part
> of this one right away since issue already exists) is if you could
> document what the values 0, 1 and 2 for new_cpl_coords[] means. 0
> means "no channel coupling", 1 means "ac3 uses channel coupling", and
> 2 has something to do with eac3 (default channel coordinates or so?),
> but I'm not quite sure what.


Sorry, yeah I should've done that before.

It is possible to determine the first block in a channel in which to
send coupling coordinates without it being explicitly encoded in the
bitstream, so the spec has a "first coupling coordinates" state variable
for each channel to avoid sending the extra bits.  The 2 is to indicate
that new coordinates need to be sent for the block but don't write the
"send new coordinates" bit to the bitstream.  I'll send a patch to
document that.

-Justin

Patch

diff --git a/libavcodec/ac3enc.c b/libavcodec/ac3enc.c
index c138f99..c61bb5e 100644
--- a/libavcodec/ac3enc.c
+++ b/libavcodec/ac3enc.c
@@ -842,9 +842,9 @@  static void count_frame_bits(AC3EncodeContext *s)
         if (block->cpl_in_use) {
             for (ch = 1; ch <= s->fbw_channels; ch++) {
                 if (block->channel_in_cpl[ch]) {
-                    if (!s->eac3 || block->new_cpl_coords != 2)
+                    if (!s->eac3 || block->new_cpl_coords[ch] != 2)
                         frame_bits++;
-                    if (block->new_cpl_coords) {
+                    if (block->new_cpl_coords[ch]) {
                         frame_bits += 2;
                         frame_bits += (4 + 4) * s->num_cpl_bands;
                     }
@@ -1370,9 +1370,9 @@  static void output_audio_block(AC3EncodeContext *s, int blk)
     if (block->cpl_in_use) {
         for (ch = 1; ch <= s->fbw_channels; ch++) {
             if (block->channel_in_cpl[ch]) {
-                if (!s->eac3 || block->new_cpl_coords != 2)
-                    put_bits(&s->pb, 1, block->new_cpl_coords);
-                if (block->new_cpl_coords) {
+                if (!s->eac3 || block->new_cpl_coords[ch] != 2)
+                    put_bits(&s->pb, 1, block->new_cpl_coords[ch]);
+                if (block->new_cpl_coords[ch]) {
                     put_bits(&s->pb, 2, block->cpl_master_exp[ch]);
                     for (bnd = 0; bnd < s->num_cpl_bands; bnd++) {
                         put_bits(&s->pb, 4, block->cpl_coord_exp [ch][bnd]);
diff --git a/libavcodec/ac3enc.h b/libavcodec/ac3enc.h
index 6d57143..c8fe0c9 100644
--- a/libavcodec/ac3enc.h
+++ b/libavcodec/ac3enc.h
@@ -123,7 +123,7 @@  typedef struct AC3Block {
     int      cpl_in_use;                        ///< coupling in use for this block     (cplinu)
     uint8_t  channel_in_cpl[AC3_MAX_CHANNELS];  ///< channel in coupling                (chincpl)
     int      num_cpl_channels;                  ///< number of channels in coupling
-    uint8_t  new_cpl_coords;                    ///< send new coupling coordinates      (cplcoe)
+    uint8_t  new_cpl_coords[AC3_MAX_CHANNELS];  ///< send new coupling coordinates      (cplcoe)
     uint8_t  cpl_master_exp[AC3_MAX_CHANNELS];  ///< coupling coord master exponents    (mstrcplco)
     int      new_snr_offsets;                   ///< send new SNR offsets
     int      new_cpl_leak;                      ///< send new coupling leak info
diff --git a/libavcodec/ac3enc_template.c b/libavcodec/ac3enc_template.c
index 103e7b1..bf72fc7 100644
--- a/libavcodec/ac3enc_template.c
+++ b/libavcodec/ac3enc_template.c
@@ -206,9 +206,10 @@  static void apply_channel_coupling(AC3EncodeContext *s)
     for (blk = 0; blk < s->num_blocks; blk++) {
         AC3Block *block  = &s->blocks[blk];
         AC3Block *block0 = blk ? &s->blocks[blk-1] : NULL;
-        int new_coords = 0;
         CoefSumType coord_diff[AC3_MAX_CHANNELS] = {0,};
 
+        memset(block->new_cpl_coords, 0, sizeof(block->new_cpl_coords));
+
         if (block->cpl_in_use) {
             /* calculate coupling coordinates for all blocks and calculate the
                average difference between coordinates in successive blocks */
@@ -233,28 +234,18 @@  static void apply_channel_coupling(AC3EncodeContext *s)
              * using coupling has changed from the previous block, or the
              * coordinate difference from the last block for any channel is
              * greater than a threshold value. */
-            if (blk == 0) {
-                new_coords = 1;
-            } else if (!block0->cpl_in_use) {
-                new_coords = 1;
+            if (blk == 0 || !block0->cpl_in_use) {
+                for (ch = 1; ch <= s->fbw_channels; ch++)
+                    block->new_cpl_coords[ch] = 1;
             } else {
                 for (ch = 1; ch <= s->fbw_channels; ch++) {
-                    if (block->channel_in_cpl[ch] && !block0->channel_in_cpl[ch]) {
-                        new_coords = 1;
-                        break;
-                    }
-                }
-                if (!new_coords) {
-                    for (ch = 1; ch <= s->fbw_channels; ch++) {
-                        if (block->channel_in_cpl[ch] && coord_diff[ch] > 0.04) {
-                            new_coords = 1;
-                            break;
-                        }
+                    if ((block->channel_in_cpl[ch] && !block0->channel_in_cpl[ch]) ||
+                        (block->channel_in_cpl[ch] && coord_diff[ch] > 0.03)) {
+                        block->new_cpl_coords[ch] = 1;
                     }
                 }
             }
         }
-        block->new_cpl_coords = new_coords;
     }
 
     /* calculate final coupling coordinates, taking into account reusing of
@@ -262,8 +253,7 @@  static void apply_channel_coupling(AC3EncodeContext *s)
     for (bnd = 0; bnd < s->num_cpl_bands; bnd++) {
         blk = 0;
         while (blk < s->num_blocks) {
-            int blk1;
-            CoefSumType energy_cpl;
+            int av_uninit(blk1);
             AC3Block *block  = &s->blocks[blk];
 
             if (!block->cpl_in_use) {
@@ -271,23 +261,18 @@  static void apply_channel_coupling(AC3EncodeContext *s)
                 continue;
             }
 
-            energy_cpl = energy[blk][CPL_CH][bnd];
-            blk1 = blk+1;
-            while (!s->blocks[blk1].new_cpl_coords && blk1 < s->num_blocks) {
-                if (s->blocks[blk1].cpl_in_use)
-                    energy_cpl += energy[blk1][CPL_CH][bnd];
-                blk1++;
-            }
-
             for (ch = 1; ch <= s->fbw_channels; ch++) {
-                CoefType energy_ch;
+                CoefSumType energy_ch, energy_cpl;
                 if (!block->channel_in_cpl[ch])
                     continue;
+                energy_cpl = energy[blk][CPL_CH][bnd];
                 energy_ch = energy[blk][ch][bnd];
                 blk1 = blk+1;
-                while (!s->blocks[blk1].new_cpl_coords && blk1 < s->num_blocks) {
-                    if (s->blocks[blk1].cpl_in_use)
+                while (!s->blocks[blk1].new_cpl_coords[ch] && blk1 < s->num_blocks) {
+                    if (s->blocks[blk1].cpl_in_use) {
+                        energy_cpl += energy[blk1][CPL_CH][bnd];
                         energy_ch += energy[blk1][ch][bnd];
+                    }
                     blk1++;
                 }
                 cpl_coords[blk][ch][bnd] = calc_cpl_coord(energy_ch, energy_cpl);
@@ -299,7 +284,7 @@  static void apply_channel_coupling(AC3EncodeContext *s)
     /* calculate exponents/mantissas for coupling coordinates */
     for (blk = 0; blk < s->num_blocks; blk++) {
         AC3Block *block = &s->blocks[blk];
-        if (!block->cpl_in_use || !block->new_cpl_coords)
+        if (!block->cpl_in_use)
             continue;
 
         clip_coefficients(&s->dsp, cpl_coords[blk][1], s->fbw_channels * 16);
@@ -313,6 +298,9 @@  static void apply_channel_coupling(AC3EncodeContext *s)
         for (ch = 1; ch <= s->fbw_channels; ch++) {
             int bnd, min_exp, max_exp, master_exp;
 
+            if (!block->new_cpl_coords[ch])
+                continue;
+
             /* determine master exponent */
             min_exp = max_exp = block->cpl_coord_exp[ch][0];
             for (bnd = 1; bnd < s->num_cpl_bands; bnd++) {
diff --git a/libavcodec/eac3enc.c b/libavcodec/eac3enc.c
index 75113b2..f3b4418 100644
--- a/libavcodec/eac3enc.c
+++ b/libavcodec/eac3enc.c
@@ -99,7 +99,7 @@  void ff_eac3_set_cpl_states(AC3EncodeContext *s)
         for (ch = 1; ch <= s->fbw_channels; ch++) {
             if (block->channel_in_cpl[ch]) {
                 if (first_cpl_coords[ch]) {
-                    block->new_cpl_coords = 2;
+                    block->new_cpl_coords[ch] = 2;
                     first_cpl_coords[ch]  = 0;
                 }
             } else {