[FFmpeg-devel] [PATCH] h264: remove clear_blocks call in threading init.

[Michael Niedermayer]

On Wed, Feb 13, 2013 at 10:37:04AM -0800, Ronald S. Bultje wrote:
> Hi,
> 
> On Tue, Feb 12, 2013 at 7:39 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > On Tue, Feb 12, 2013 at 06:30:15PM +0100, Michael Niedermayer wrote:
> >> On Tue, Feb 12, 2013 at 09:18:34AM -0800, Ronald S. Bultje wrote:
> >> > Hi,
> >> >
> >> > On Tue, Feb 12, 2013 at 9:05 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
> >> > > On Mon, Feb 11, 2013 at 06:08:45PM -0800, Ronald S. Bultje wrote:
> >> > >> From: "Ronald S. Bultje" <rsbultje at gmail.com>
> >> > >>
> >> > >> Init code in that if statement goes down from 26716 cycles to 26047
> >> > >> cycles, i.e. the removal of the clear_blocks and smaller memcpy()
> >> > >> together save around 670 cycles.
> >> > >> ---
> >> > >>  libavcodec/h264.c | 7 +++----
> >> > >>  1 file changed, 3 insertions(+), 4 deletions(-)
> >> > >>
> >> > >> diff --git a/libavcodec/h264.c b/libavcodec/h264.c
> >> > >> index 2d6c5e6..994e002 100644
> >> > >> --- a/libavcodec/h264.c
> >> > >> +++ b/libavcodec/h264.c
> >> > >> @@ -1253,7 +1253,9 @@ static int decode_update_thread_context(AVCodecContext *dst,
> >> > >>
> >> > >>          // copy all fields after MpegEnc
> >> > >>          memcpy(&h->s + 1, &h1->s + 1,
> >> > >> -               sizeof(H264Context) - sizeof(MpegEncContext));
> >> > >> +               offsetof(H264Context, intra_gb) - sizeof(MpegEncContext));
> >> > >> +        memcpy(&h->cabac, &h1->cabac,
> >> > >> +               sizeof(H264Context) - offsetof(H264Context, cabac));
> >> > >>          memset(h->sps_buffers, 0, sizeof(h->sps_buffers));
> >> > >>          memset(h->pps_buffers, 0, sizeof(h->pps_buffers));
> >> > >>
> >> > >
> >> > > If either of these fields is moved in the structure or a new field
> >> > > added between then threading can mysteriously break.
> >> > > also the comment above would not match the code anymore
> >> > > iam not sure this is a good idea but if it is done then the fields
> >> > > should be documented in the struct so a developer touching them would
> >> > > know they are special ...
> >> >
> >> > Isn't that the case already? The non-init code in
> >> > update_thread_context() for mpeg and h264 100% depends on ordering of
> >> > the fields, any field ordering change will break the threading.
> >>
> >> yes, the problem exists already with some fields
> >
> > also what i forgot, iam happy to add these comments/documentation to
> > the struct/fields myself if you prefer ...
> 
> Yeah that'd be helpful, thanks.

i found a better solution than documentation
applied with that

thanks


> 
> (And yes I sometimes fall off the radar on weekdays - work = busy.)
> 
> Ronald
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I have never wished to cater to the crowd; for what I know they do not
approve, and what they approve I do not know. -- Epicurus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130214/88cca0d4/attachment.asc>