[FFmpeg-soc] [soc]: r5067 - spdif/spdif.c

[Bartlomiej Wolowiec]

Thursday 13 August 2009 12:05:10 Reimar Döffinger napisał(a):
> On Thu, Aug 13, 2009 at 11:10:21AM +0200, bwolowiec wrote:
> > Modified: spdif/spdif.c
> > =========================================================================
> >===== --- spdif/spdif.c	Thu Aug 13 10:28:04 2009	(r5066)
> > +++ spdif/spdif.c	Thu Aug 13 11:10:21 2009	(r5067)
> > @@ -29,6 +29,8 @@
> >  #include "libavcodec/ac3.h"
> >  #include "libavcodec/dca.h"
> >  #include "libavcodec/aac_parser.h"
> > +#define __USE_XOPEN
> > +#include <unistd.h>
>
> That needs a feature test, but I think it would be better to add a
> DSPUtil function, similar to bswap_buf

Ok, but dsputil_init currently needs pointer to AVCodecContext... so maybe 
best solution is just to make separate function, which can be changed in the 
future...

> > @@ -55,6 +57,10 @@ typedef struct IEC958Context {
> >      int pkt_size;               ///< Length code (number of bits or
> > bytes - according to data_type) int pkt_offset;             ///<
> > Repetition period of a data burst in bytes int (*header_info)
> > (AVFormatContext *s, AVPacket *pkt);
> > +#if !HAVE_BIGENDIAN
> > +    uint8_t *buffer;
> > +    int buffer_size;
> > +#endif
>
> I'd be in favour of leaving this in unconditionally, even if it is
> strictly speaking useless bloat on big endian.

hmm.. so, what are the pros of leaving this unconditionally?

>[...]
> > @@ -237,13 +256,13 @@ static int spdif_write_packet(struct AVF
> >  #if HAVE_BIGENDIAN
> >      put_buffer(s->pb, pkt->data, pkt->size & ~1);
> >  #else
> > -    {
> > -        //XXX swab... ?
> > -        uint16_t *data = (uint16_t *) pkt->data;
> > -        int i;
> > -        for (i = 0; i < pkt->size >> 1; i++)
> > -            put_be16(s->pb, data[i]);
> > +    if (ctx->buffer_size < pkt->size) {
> > +        av_fast_malloc(&ctx->buffer, &ctx->buffer_size, pkt->size +
> > FF_INPUT_BUFFER_PADDING_SIZE); +        if (!ctx->buffer)
> > +            return AVERROR(ENOMEM);
> >      }
> > +    swab(pkt->data, ctx->buffer, pkt->size & ~1);
> > +    put_buffer(s->pb, ctx->buffer, pkt->size & ~1);
> >  #endif
>
> Btw. using
> if (HAVE_BIGENDIAN) {
> ...
> } else {
> }
>
> will ensure that compilation of both paths is checked on all builds, and
> unless someone compiles with -O0 results in the same code.

so why in other places in FFmpeg there are #if ? any pros of this ?
-- 
Bartlomiej Wolowiec