[FFmpeg-devel] [PATCH] MPEG-4 Parametric Stereo decoder
Michael Niedermayer
michaelni
Wed Jun 9 04:10:38 CEST 2010
On Tue, Jun 08, 2010 at 02:13:41PM -0400, Alex Converse wrote:
> On Thu, Jun 3, 2010 at 12:47 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > On Wed, Jun 02, 2010 at 10:28:32PM -0400, Alex Converse wrote:
> > > On Mon, May 31, 2010 at 6:42 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > [...]
> > > >
> > > > > +{
> > > > > + ? ?int b;
> > > > > + ? ?int table_idx = dt ? huff_icc_dt : huff_icc_df;
> > > > > + ? ?VLC_TYPE (*vlc_table)[2] = vlc_ps[table_idx].table;
> > > > > + ? ?if (dt) {
> > > > > + ? ? ? ?int e_prev = e ? e - 1 : ps->num_env_old - 1;
> > > > > + ? ? ? ?e_prev = FFMAX(e_prev, 0);
> > > > > + ? ? ? ?for (b = 0; b < ps->nr_icc_par; b++) {
> > > > > + ? ? ? ? ? ?ps->icc_par[e][b] = ps->icc_par[e_prev][b] + get_vlc2(gb, vlc_table, 9, 3) - huff_offset[table_idx];
> > > > > + ? ? ? ? ? ?if (ps->icc_par[e][b] > 7U) {
> > > > > + ? ? ? ? ? ? ? ?av_log(avctx, AV_LOG_ERROR, "illegal icc\n");
> > > > > + ? ? ? ? ? ? ? ?return -1;
> > > > > + ? ? ? ? ? ?}
> > > > > + ? ? ? ?}
> > > > > + ? ?} else {
> > > > > + ? ? ? ?int prev = 0;
> > > > > + ? ? ? ?for (b = 0; b < ps->nr_icc_par; b++) {
> > > > > + ? ? ? ? ? ?prev += get_vlc2(gb, vlc_table, 9, 3) - huff_offset[table_idx];
> > > > > + ? ? ? ? ? ?ps->icc_par[e][b] = prev;
> > > > > + ? ? ? ? ? ?if (ps->icc_par[e][b] > 7U) {
> > > > > + ? ? ? ? ? ? ? ?av_log(avctx, AV_LOG_ERROR, "illegal icc\n");
> > > > > + ? ? ? ? ? ? ? ?return -1;
> > > > > + ? ? ? ? ? ?}
> > > > > + ? ? ? ?}
> > > > > + ? ?}
> > > >
> > > > this could be simplified to:
> > > > for (b = 0; b < ps->nr_icc_par; b++) {
> > > > ? ?ps->icc_par[e][b]= prev[b] + get_vlc2(gb, vlc_table, 9, 3) - huff_offset[table_idx];
> > > > ? ?if (ps->icc_par[e][b] > 7U) {
> > > > ? ? ? ?av_log(avctx, AV_LOG_ERROR, "illegal icc\n");
> > > > ? ? ? ?return -1;
> > > > ? ?}
> > > > }
> > > > and this possibly applies to other functions as well
> > > >
> > >
> > > Are you talking about the if case the else case or both combined?
> >
> > iam talking about combining the if and else by having prev be a pointer
> > into par
> >
>
> That would require extra padding for the df case. It would also add
> more array lookups.
>
> >
> > > why is prev dereferenced two levels but not par?
> > > Am I missing something here?
> > >
> > >
> > > > > + ? ?return 0;
> > > > > +}
> > > > > +
> > > > > +static void ipd_data(GetBitContext *gb, PSContext *ps, int e, int dt)
> > > > > +{
> > > > > + ? ?int b;
> > > > > + ? ?int table_idx = dt ? huff_ipd_dt : huff_ipd_df;
> > > > > + ? ?VLC_TYPE (*vlc_table)[2] = vlc_ps[table_idx].table;
> > > > > + ? ?if (dt) {
> > > > > + ? ? ? ?int e_prev = e ? e - 1 : ps->num_env_old - 1;
> > > > > + ? ? ? ?e_prev = FFMAX(e_prev, 0);
> > > > > + ? ? ? ?for (b = 0; b < ps->nr_ipdopd_par; b++) {
> > > > > + ? ? ? ? ? ?ps->ipd_par[e][b] = (ps->ipd_par[e_prev][b] + get_vlc2(gb, vlc_table, 9, 1)) & 0x07;
> > > > > + ? ? ? ?}
> > > > > + ? ?} else {
> > > > > + ? ? ? ?int prev = 0;
> > > > > + ? ? ? ?for (b = 0; b < ps->nr_ipdopd_par; b++) {
> > > > > + ? ? ? ? ? ?prev += get_vlc2(gb, vlc_table, 9, 3);
> > > > > + ? ? ? ? ? ?prev &= 0x07;
> > > > > + ? ? ? ? ? ?ps->ipd_par[e][b] = prev;
> > > > > + ? ? ? ?}
> > > > > + ? ?}
> > > > > +}
> > > > > +
> > > > > +static void opd_data(GetBitContext *gb, PSContext *ps, int e, int dt)
> > > > > +{
> > > > > + ? ?int b;
> > > > > + ? ?int table_idx = dt ? huff_opd_dt : huff_opd_df;
> > > > > + ? ?VLC_TYPE (*vlc_table)[2] = vlc_ps[table_idx].table;
> > > > > + ? ?if (dt) {
> > > > > + ? ? ? ?int e_prev = e ? e - 1 : ps->num_env_old - 1;
> > > > > + ? ? ? ?e_prev = FFMAX(e_prev, 0);
> > > > > + ? ? ? ?for (b = 0; b < ps->nr_ipdopd_par; b++) {
> > > > > + ? ? ? ? ? ?ps->opd_par[e][b] = (ps->opd_par[e_prev][b] + get_vlc2(gb, vlc_table, 9, 1)) & 0x07;
> > > > > + ? ? ? ?}
> > > > > + ? ?} else {
> > > > > + ? ? ? ?int prev = 0;
> > > > > + ? ? ? ?for (b = 0; b < ps->nr_ipdopd_par; b++) {
> > > > > + ? ? ? ? ? ?prev += get_vlc2(gb, vlc_table, 9, 3);
> > > > > + ? ? ? ? ? ?prev &= 0x07;
> > > > > + ? ? ? ? ? ?ps->opd_par[e][b] = prev;
> > > > > + ? ? ? ?}
> > > > > + ? ?}
> > > > > +}
> > > >
> > > > all these functions look like they do pretty much the same, maybe this can be
> > > > done with just a single function?
> > > >
> > >
> > > I thought about it but they all have different valid ranges, unless
> > > you just mean merging ipd/opd which both have the same circular range.
> >
> > why not pass the range as argument to the function?
> >
>
> The three range cases seem vastly different:
>
> FFABS(ps->iid_par[e][b]) > 7 + 8 * ps->iid_quant
> ps->icc_par[e][b] > 7U
> prev &= 0x07
>
> the last one even using circular addressing.
a maximum, a minimum and a &C
or a condition passed as argument into a macro
[...]
> > > > > + ? ? ? ? ? ?}
> > > > > + ? ? ? ? ? ?temp[ssb][0] = sum_re;
> > > > > + ? ? ? ? ? ?temp[ssb][1] = sum_im;
> > > > > + ? ? ? ?}
> > > > > + ? ? ? ?out[0][i][0] = temp[6][0];
> > > > > + ? ? ? ?out[0][i][1] = temp[6][1];
> > > > > + ? ? ? ?out[1][i][0] = temp[7][0];
> > > > > + ? ? ? ?out[1][i][1] = temp[7][1];
> > > > > + ? ? ? ?out[2][i][0] = temp[0][0];
> > > > > + ? ? ? ?out[2][i][1] = temp[0][1];
> > > > > + ? ? ? ?out[3][i][0] = temp[1][0];
> > > > > + ? ? ? ?out[3][i][1] = temp[1][1];
> > > > > + ? ? ? ?out[4][i][0] = temp[2][0] + temp[5][0];
> > > > > + ? ? ? ?out[4][i][1] = temp[2][1] + temp[5][1];
> > > > > + ? ? ? ?out[5][i][0] = temp[3][0] + temp[4][0];
> > > > > + ? ? ? ?out[5][i][1] = temp[3][1] + temp[4][1];
> > > > > + ? ?}
> > > > > +}
> > > > > +
> > > > > +static av_noinline void hybrid4_8_12_cx(float (*in)[2], float (*out)[32][2], const float (*filter)[7][2], int N, int len)
> > > > > +{
> > > > > + ? ?int i, j, ssb;
> > > > > +
> > > > > + ? ?for (i = 0; i < len; i++) {
> > > > > + ? ? ? ?for (ssb = 0; ssb < N; ssb++) {
> > > > > + ? ? ? ? ? ?float sum_re = filter[ssb][6][0] * in[i+6][0], sum_im = filter[ssb][6][0] * in[i+6][1];
> > > > > + ? ? ? ? ? ?for (j = 0; j < 6; j++) {
> > > > > + ? ? ? ? ? ? ? ?float in0_re = in[i+j][0];
> > > > > + ? ? ? ? ? ? ? ?float in0_im = in[i+j][1];
> > > > > + ? ? ? ? ? ? ? ?float in1_re = in[i+12-j][0];
> > > > > + ? ? ? ? ? ? ? ?float in1_im = in[i+12-j][1];
> > > > > + ? ? ? ? ? ? ? ?sum_re += filter[ssb][j][0] * (in0_re + in1_re) - filter[ssb][j][1] * (in0_im - in1_im);
> > > > > + ? ? ? ? ? ? ? ?sum_im += filter[ssb][j][0] * (in0_im + in1_im) + filter[ssb][j][1] * (in0_re - in1_re);
> > > > > + ? ? ? ? ? ?}
> > > > > + ? ? ? ? ? ?out[ssb][i][0] = sum_re;
> > > > > + ? ? ? ? ? ?out[ssb][i][1] = sum_im;
> > > > > + ? ? ? ?}
> > > > > + ? ?}
> > > > > +}
> > > > > +
> > > > > +static av_noinline void hybrid_analysis(float out[91][32][2], float in[64][44][2], int is34, int len)
> > > > > +{
> > > > > + ? ?int i;
> > > > > + ? ?if(is34) {
> > > > > + ? ? ? ?hybrid4_8_12_cx(in[0], out, ? ?f34_0_12, 12, len);
> > > > > + ? ? ? ?hybrid4_8_12_cx(in[1], out+12, f34_1_8, ? 8, len);
> > > > > + ? ? ? ?hybrid4_8_12_cx(in[2], out+20, f34_2_4, ? 4, len);
> > > > > + ? ? ? ?hybrid4_8_12_cx(in[3], out+24, f34_2_4, ? 4, len);
> > > > > + ? ? ? ?hybrid4_8_12_cx(in[4], out+28, f34_2_4, ? 4, len);
> > > > > + ? ? ? ?for (i = 0; i < 59; i++) {
> > > > > + ? ? ? ? ? ?memcpy(out[32 + i], in[5 + i]+6, len * sizeof(in[0][0]));
> > > > > + ? ? ? ?}
> > > > > + ? ?} else {
> > > > > + ? ? ? ?hybrid6_cx(in[0], out, f20_0_8, len);
> > > > > + ? ? ? ?hybrid2_re(in[1], out+6, g1_Q2, len, 1);
> > > > > + ? ? ? ?hybrid2_re(in[2], out+8, g1_Q2, len, 0);
> > > > > + ? ? ? ?for (i = 0; i < 61; i++) {
> > > > > + ? ? ? ? ? ?memcpy(out[10 + i], in[3 + i]+6, len * sizeof(in[0][0]));
> > > > > + ? ? ? ?}
> > > >
> > > > is all that memcpy unavoidable?
> > > >
> > >
> > > It may be avoidable with a float(**)x[2] instead of a float(*)[][2].
> > > FFmpeg seems to avoid the former.
> >
> > we prefer 2d fixed size arrays over pointer array due to extra dereferences
> >
>
> But are pointer matrices preferred when they avoid large memcpys?
faster is preferred, whatver that is
if speed is equal, simpler is preferred
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
In fact, the RIAA has been known to suggest that students drop out
of college or go to community college in order to be able to afford
settlements. -- The RIAA
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100609/820ad5c4/attachment.pgp>
More information about the ffmpeg-devel
mailing list