[Ffmpeg-devel] MPEG-2 Acceleration Under Mac OS X - Can We Move This Forward?

[John Dalgliesh]

Hi Nigel,

On Wed, 30 Aug 2006, Nigel Pearson wrote:
>
>> I didn't come up with a solution that doesn't either:
>> 1. Add lots of #ifdefs uglifying the code (and preventing runtime 
>> selection), or
>> 2. Add lots of ifs slowing down the code, or
>> 3. Duplicate significant portions of the code.
>
> 	Here are my slight changes to your mpeg12.c patches.

They are a lot more than slight! Maybe they are slight vs someone else's 
modification of the patch.

> My understanding of that file, (or how Michael N. would
> like to factor out the hardware accel. stuff) isn't great,
> but I hope this can be a start for discussion/review.

I don't think it will be reviewed unless you at least follow Michael's 
suggestion of collapsing the _fast methods. Using diff it should not be 
hard to merge them correctly.

> 	In terms of the concerns above:
> 1) There are lots of #ifdefs, but not many
> more than the existing HAVE_XVMC blocks?
> 2) Similarly with the ifs for runtime selection.

But they are in a (more) inner loop.

> 3) I tried to move the large blocks into a separate file,
> but there are still lots of sections that are in similar
> locations to the XVMC code.

IMHO the two fns you moved aren't large enough to warrant this.

> 	I think we also need to settle on one namespace.
> Either DVDV or Accellent (at the moment there are both).

I never used 'accellent' in my ffmpeg patch, someone else must have done 
that. Accellent is just the silly name of the demo app, DVDV(ideo) is the 
correct name for the acceleration method.

> (Note that this also includes some other MythTV additions-
> VIA XVMC hardware accel, and a closed captioning decoder)

A patch with multiple unrelated changes like this will never be accepted. 
I know you're just putting it out there for discussion, but you should 
make ppl's life easier if you can.

Other quick comments:
- The dct elts should not be set into the blocks for sanity in real 
code; that was just for testing.
- The way the stream pointers are accessed appears inefficient.
- Don't need two vars in the avctx, just have the pointer NULL if not 
enabled.

{P^/