[FFmpeg-devel] Request for review
Michael Niedermayer
michaelni
Thu Oct 23 12:05:59 CEST 2008
On Wed, Oct 22, 2008 at 06:03:36PM -0700, Roman V. Shaposhnik wrote:
> On Tue, 2008-10-21 at 19:40 +0200, Michael Niedermayer wrote:
> > > @@ -384,10 +401,20 @@ static inline void dv_decode_video_segment(DVVideoContext *s,
> > > assert((((int)mb_bit_buffer)&7)==0);
> > > assert((((int)vs_bit_buffer)&7)==0);
> > >
> > > + j = buf_offset/150;
> > > + chan = j/s->sys->difseg_size;
> > > + seg = j%s->sys->difseg_size;
> > > + slot = ((buf_offset%150)*15 - 90)/(5<<4);
> >
> > I think this could be put in dv_anchor or referenced by it,
>
> I've thought about exactly the same thing. But! If we do
> that we would have to potentially regenerate dv_anchor
> every time there's a change of DV profile midstream.
> Would that be more acceptable?
If dv_anchor is in the context, allocated by avmalloc() then it should be
no problem.
>
> > > + /* in 1080i50 and 720p50 some seq are unused */
> > > + if ((DV_PROFILE_IS_1080i50(s->sys) && chan != 0 && seg == 11) ||
> > > + (DV_PROFILE_IS_720p50(s->sys) && seg > 9))
> > > + return;
> >
> > Can these not just be ommited from dv_anchor ?
>
> See above.
>
> > > @@ -982,48 +1006,14 @@ static inline void dv_encode_video_segment(DVVideoContext *s,
> > >
> > > static int dv_decode_mt(AVCodecContext *avctx, void* sl)
> > > {
> > > - DVVideoContext *s = avctx->priv_data;
> > > - int slice = (size_t)sl;
> > > -
> > > - /* which DIF channel is this? */
> > > - int chan = slice / (s->sys->difseg_size * 27);
> > > -
> > > - /* slice within the DIF channel */
> > > - int chan_slice = slice % (s->sys->difseg_size * 27);
> > > -
> > > - /* byte offset of this channel's data */
> > > - int chan_offset = chan * s->sys->difseg_size * 150 * 80;
> > > -
> > > - /* DIF sequence */
> > > - int seq = chan_slice / 27;
> > > -
> > > - /* in 1080i50 and 720p50 some seq are unused */
> > > - if ((DV_PROFILE_IS_1080i50(s->sys) && chan != 0 && seq == 11) ||
> > > - (DV_PROFILE_IS_720p50(s->sys) && seq > 9))
> > > - return 0;
> > > -
> > > - dv_decode_video_segment(s, &s->buf[(seq*6+(chan_slice/3)+chan_slice*5+7)*80 + chan_offset],
> > > - &s->sys->video_place[slice*5]);
> > > + dv_decode_video_segment((DVVideoContext *)avctx->priv_data, (size_t)sl);
> > > return 0;
> > > }
> >
> > after this change dv_decode_mt() could maybe be ommited and
> > dv_decode_video_segment() be used directly
>
> Absolutely! But I thought you would be happier with that change
> in a separate patch.
yes, indeed, iam happier with a seperate change :)
> Please let me know if you want me to
> roll it into this one.
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Democracy is the form of government in which you can choose your dictator
-------------- 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/20081023/7987b5ff/attachment.pgp>
More information about the ffmpeg-devel
mailing list