[FFmpeg-devel] [PATCH] Demuxer for Leitch/Harris' VR native stream format (LXF)
Tomas Härdin
tomas.hardin
Mon Sep 13 10:48:35 CEST 2010
On Fri, 2010-09-10 at 13:43 +0200, Michael Niedermayer wrote:
> On Tue, Sep 07, 2010 at 04:17:10PM +0200, Tomas H?rdin wrote:
> > > > +
> > > > + if (!memcmp(p->buf, LXF_IDENT, LXF_IDENT_LENGTH))
> > > > + return AVPROBE_SCORE_MAX;
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > >
> > > > +//returns zero if checksum is OK, non-zero otherwise
> > >
> > > doxy
> >
> > Why? It's not exported. Or did you mean the comment style in general? I
> > doxyfied check_checksum() and num_set_bits() as an example.
>
> we are trying to have all comments in doxy format where its applicable
> and it is for documenting a function even if internal
Fair enough. Documented the rest of them, except for the ones that go in
AVInputFormat.
> > > > +//returns number of bits set in value
> > > > +static int num_set_bits(uint32_t value) {
> > > > + int ret;
> > > > +
> > > > + for(ret = 0; value; ret += (value & 1), value >>= 1);
> > > > +
> > > > + return ret;
> > > > +}
> > >
> > > if we dont have a population count function yet, than one should be added
> > > to some header in libavutil
> >
> > I couldn't find one. That probably belongs in its own thread though.
> >
> > Which files would such a function belong in - intmath.h/c, common.h or
> > somewhere else? Also, which name would be best: ff_count_bits(),
> > av_count_bits() or something else?
>
> av_popcount()
> would be similar to gccs __builtin_popcount()
OK. I attached popcount.patch which adds such a function to common.h.
Also bumped minor of lavu. The implementation uses a 16-byte LUT and
therefore counts four bits at a time. I suspect there are better
solutions though. I did verify that it returns exactly the same number
the other implementation does for all 2^32 possible input values.
A compiler version check could use the builtin, but I'll leave that to
someone more knowledgable.
> [...]
> > +//reads and checksums packet header. returns format and size of payload
> > +static int get_packet_header(AVFormatContext *s, unsigned char *header,
> > + uint32_t *format)
> > +{
> > + LXFDemuxContext *lxf = s->priv_data;
> > + ByteIOContext *pb = s->pb;
> > + int size, track_size, samples;
> > + AVStream *st;
> > +
> > + if (get_buffer(pb, header, LXF_PACKET_HEADER_SIZE) != LXF_PACKET_HEADER_SIZE)
> > + return AVERROR(EIO);
> > +
> > + if (memcmp(header, LXF_IDENT, LXF_IDENT_LENGTH)) {
> > + av_log(s, AV_LOG_ERROR, "packet ident mismatch - out of sync?\n");
> > + return -1;
> > + }
> > +
> > + if (check_checksum(header))
> > + av_log(s, AV_LOG_ERROR, "checksum error\n");
> > +
> > + *format = AV_RL32(&header[32]);
> > + size = AV_RL32(&header[36]);
> > +
> > + //type
> > + switch (AV_RL32(&header[16])) {
> > + case 0:
> > + //video
> > + //skip VBI data and metadata
> > + url_fskip(pb, AV_RL32(&header[44]) + AV_RL32(&header[52]));
>
> cant this lead to a backward seek and thus infinite loop ?
Yes, assuming AV_RL32() returns signed. Fixed by casting to uint32_t and
splitting up into two skips.
> > +
> > + break;
> > + case 1:
> > + //audio
> > + if (!(st = s->streams[1])) {
> > + av_log(s, AV_LOG_INFO, "got audio packet, but no audio stream present\n");
> > + break;
> > + }
> > +
> > + //set codec based on specified audio bitdepth
> > + //we only support tightly packed 16-, 20-, 24- and 32-bit PCM at the moment
> > + *format = AV_RL32(&header[40]);
> > + lxf->bps = (*format >> 6) & 0x3F;
> > +
> > + if (lxf->bps != (*format & 0x3F)) {
> > + av_log(s, AV_LOG_WARNING, "only tightly packed PCM currently supported\n");
> > + return AVERROR_PATCHWELCOME;
> > + }
> > +
> > + if (lxf->bps != 16 && lxf->bps != 20 && lxf->bps != 24 && lxf->bps != 32) {
> > + av_log(s, AV_LOG_WARNING,
> > + "only 16-, 20-, 24- and 32-bit PCM currently supported\n");
> > + return AVERROR_PATCHWELCOME;
> > + }
> > +
> > + //round bps up to next multiple of eight
> > + if (lxf->bps <= 16) {
> > + st->codec->codec_id = CODEC_ID_PCM_S16LE;
> > + st->codec->bits_per_coded_sample = 16;
> > + } else if (lxf->bps <= 24) {
> > + st->codec->codec_id = CODEC_ID_PCM_S24LE;
> > + st->codec->bits_per_coded_sample = 24;
> > + } else {
> > + st->codec->codec_id = CODEC_ID_PCM_S32LE;
> > + st->codec->bits_per_coded_sample = 32;
> > + }
> > +
> > + track_size = AV_RL32(&header[48]);
> > + samples = track_size * 8 / lxf->bps;
> > +
> > + //use audio packet size to determine video standard
> > + //for NTSC we have one 8008-sample audio frame per five video frames
> > + if (samples == LXF_SAMPLERATE * 5005 / 30000) {
> > + av_set_pts_info(s->streams[0], 64, 1001, 30000);
> > + s->duration = av_rescale_q(lxf->num_frames,
> > + (AVRational){1001,30000}, AV_TIME_BASE_Q);
> > + } else {
> > + //assume PAL, but warn if we don't have 1920 samples
> > + if (samples != LXF_SAMPLERATE / 25)
> > + av_log(s, AV_LOG_WARNING,
> > + "video doesn't seem to be PAL or NTSC. guessing PAL\n");
> > +
> > + av_set_pts_info(s->streams[0], 64, 1, 25);
> > + s->duration = av_rescale_q(lxf->num_frames,
> > + (AVRational){1,25}, AV_TIME_BASE_Q);
> > + }
>
> you should set the duration of the AVStream and let the s->duration for the
> core to fill (should be simpler)
Cool - didn't know that. Code much simplified.
> [...]
> > +static int lxf_read_packet(AVFormatContext *s, AVPacket *pkt)
> > +{
> > + LXFDemuxContext *lxf = s->priv_data;
> > + ByteIOContext *pb = s->pb;
> > + unsigned char header[LXF_PACKET_HEADER_SIZE];
> > + int ret, stream, ofs = 0, format;
> > + AVStream *ast = NULL;
> > +
> > + if ((ret = get_packet_header(s, header, &format)) < 0)
> > + return ret;
> > +
> > + av_log(s, AV_LOG_DEBUG, "got %d B packet\n", ret);
> > +
> > + if ((stream = AV_RL32(&header[16])) == 1 && !(ast = s->streams[stream])) {
> > + av_log(s, AV_LOG_ERROR, "got audio packet without having an audio stream\n");
> > + return -1;
> > + }
> > +
> > + if (ast) {
> > + if (lxf->bps == 20) {
> > + //20-bit PCM gets inplace-expanded to 24-bit
> > + //make room for 20% more data in the packet than we read
> > + ofs = ret / 5;
> > + }
> > +
> > + //make sure the data fits in the buffer
> > + if (ofs + ret > LXF_MAX_AUDIO_PACKET) {
> > + av_log(s, AV_LOG_ERROR, "audio packet too large (%i > %i)\n",
> > + ofs + ret, LXF_MAX_AUDIO_PACKET);
> > + return -1;
>
> ofs + ret can overflow
Fixed.
Two patches attached. lxfdec3.patch uses av_popcount() and therefore
depends on popcount.patch.
/Tomas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: lxfdec3.patch
Type: text/x-patch
Size: 14918 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100913/60363bc1/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: popcount.patch
Type: text/x-patch
Size: 1441 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100913/60363bc1/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100913/60363bc1/attachment.pgp>
More information about the ffmpeg-devel
mailing list