[Ffmpeg-devel] Re: Bethsoft VID demuxer and decoder
Michael Niedermayer
michaelni
Tue Apr 3 23:40:21 CEST 2007
Hi
On Tue, Apr 03, 2007 at 12:43:13AM -0700, Nicholas T wrote:
> Sorry I don't have a lot of time...I still have to do the audio pts
> and stack revision.
if you dont have a lot of time now how do we know that you have a lot
of time for the actual SOC?
>
> Do you have buffer wrapper functions for allocation / deallocation you
> want me to use for the stack revision?
> The heap is better for large
> allocations?
yes
> I don't see how the multiplication can overflow; sorry.
> Could you be more specific?
i dont know how i can be more specific than "overflowing multiplication"
> The numbers are 256 or 320 for the width,
> multiplied by 200 for the height.
no they are not, they are what is stored in the file
> Are you talking about integer
> overflow or memory overflow?
integer overflow followed by writing to unallocated memory
>
> On 3/28/07, Michael Niedermayer <michaelni at gmx.at> wrote:
> >Hi
> >
> >On Tue, Mar 27, 2007 at 06:25:40PM -0700, Nicholas T wrote:
> >[...]
> >
> >> >return -1
> >> done, and with the invalid starting block, the av_log is with ERROR
> >> instead of VERBOSE.
> >
> >the code will not work with negative linesize
>
> okay, all linesizes < width now fail, as that's rather meaningless.
its not meaningless, the image can be flipped and data[0] can point to
the last line
[...]
> >AV_RB24()
>
> This doesn't multiply by 4.
AV_RB24()*4 does
[...]
> >why duplicate AVCodecContext width/height?
>
> The demuxer needs it, because, as you put it, Bethsoft VID is
> misdesigned, and doens't have a simple "video frame length" field that
> would have reduced a lot of ugly code and trouble... AVFormatContext
> doesn't have width/height.
it does have AVStream which has AVCodecContext which has width/height
[...]
> + /// main code
> + do
i dont think doxygen supports comments before loops
> + {
> + rle_num_bytes = buf++[0];
> +
> + // if difference frame, skip rle_num_bytes, else set rle_num_bytes to the next byte
> + if(rle_num_bytes > 0x80)
> + {
> + rle_num_bytes -= 0x80;
> + // the last (ugly) term compensates for skipping over the last few bytes in each line
> + if(line_start + xoffset + rle_num_bytes + ((xoffset + rle_num_bytes) / avctx->width) *
> + (vid->frame.linesize[0] - avctx->width) > frame_end) { return -1; }
> + for(length = rle_num_bytes; length >= 0;) // allow to run when length = 0 to increment buffer if full frame
> + {
> + // no overflow to next line
> + if(xoffset + length < avctx->width)
> + {
> + if(block_type == VIDEO_FULL_FRAME_BLOCK) { memset(&line_start[xoffset], buf++[0], length); }
> + xoffset += length;
> + break;
> + }
> +
> + // copy any bytes starting at the current position, and ending at the frame width
> + if(block_type == VIDEO_FULL_FRAME_BLOCK) { memset(&line_start[xoffset], buf[0], avctx->width - xoffset); }
> + line_start += vid->frame.linesize[0]; // start at the next line next loop iteration
> + length -= (avctx->width - xoffset); // decrement the number of bytes remaining
> + xoffset = 0;
> + }
> + }
> +
> + // plain sequence of bytes, same for all video formats
> + else if(rle_num_bytes != 0)
> + {
> + length = rle_num_bytes;
> + // the last (ugly) term compensates for skipping over the last few bytes in each line
> + if(line_start + xoffset + rle_num_bytes + ((xoffset + rle_num_bytes) / avctx->width) *
> + (vid->frame.linesize[0] - avctx->width) > frame_end) { return -1; }
> + for(length = rle_num_bytes; length > 0;)
> + {
> + // no overflow to next line
> + if(xoffset + length < avctx->width)
> + {
> + bytestream_get_buffer(&buf, &line_start[xoffset], length);
> + xoffset += length;
> + break;
> + }
> +
> + // copy any bytes starting at the current position, and ending at the frame width
> + bytestream_get_buffer(&buf, &line_start[xoffset], avctx->width - xoffset);
> + line_start += vid->frame.linesize[0]; // start at the next line next loop iteration
> + length -= (avctx->width - xoffset); // decrement the number of bytes remaining
> + xoffset = 0;
> + }
> + }
> + // done with the frame, don't need (and sometimes don't get) terminating zero
> + if(line_start + vid->frame.linesize[0] > frame_end && xoffset == avctx->width) { break; }
> + } while(rle_num_bytes);
significantly too complex for a rle decoder
[...]
> +typedef struct BVID_DemuxContext
> +{
> + struct {
> + int frame_width, frame_height, npixels;
> + int nframes;
> + /** delay value between frames, added to individual frame delay.
> + * custom units, which will be added to other custom units (~=16ms according
> + * to free, unofficial documentation) */
> + int bethsoft_global_delay;
> + } header;
whats that nested struct good for?
> +
> + /** video presentation time stamp.
> + * delay = 16 milliseconds * (global_delay + per_frame_delay) */
> + int64_t video_pts;
> +
> + /// same as video section, but individual delays are only added for the first frame.
> + int64_t audio_pts;
> +
> + int is_finished;
> +
> +} BVID_DemuxContext;
the demux context belongs into the demuxer
> +
> +typedef struct BethsoftvidContext {
> + AVCodecContext *avctx;
> + AVFrame frame;
> +} BethsoftvidContext;
the decode context belongs into the decoder
[...]
> Index: libavcodec/avcodec.h
> ===================================================================
> --- libavcodec/avcodec.h (revision 8603)
> +++ libavcodec/avcodec.h (working copy)
> @@ -158,6 +158,7 @@
> CODEC_ID_FFH264,
> CODEC_ID_DXA,
> CODEC_ID_DNXHD,
> + CODEC_ID_BETHSOFTVID,
> CODEC_ID_THP,
breaks ABI
[...]
> + BVID_DemuxContext *vid = (BVID_DemuxContext *)s->priv_data; // permanent data outside of function
useless cast
[...]
> + if (get_buffer(pb, scratch, 15) != 15)
> + return AVERROR_IO;
> + vid->header.nframes = AV_RL16(&scratch[5]);
> + vid->header.frame_width = AV_RL16(&scratch[7]);
> + vid->header.frame_height = AV_RL16(&scratch[9]);
> + vid->header.bethsoft_global_delay = AV_RL16(&scratch[11]);
why not use get_le16(pb); ?
[...]
> + av_set_pts_info(stream, 16, 1, 60); // 16 ms increments, i.e. 60 fps
why 16bit ?
[...]
> + stream->codec->codec_type = CODEC_TYPE_AUDIO;
> + stream->codec->codec_id = CODEC_ID_PCM_U8;
> + stream->codec->channels = 1;
> + stream->codec->sample_rate = 11025;
> + stream->codec->bits_per_sample = 8;
> + stream->codec->bit_rate = stream->codec->channels * stream->codec->sample_rate * stream->codec->bits_per_sample;
> + stream->codec->width = vid->header.frame_width;
> + stream->codec->height = vid->header.frame_height;
audio has no width/height
[...]
> + if(get_buffer(pb, vidbuf, rle_num_bytes) != rle_num_bytes)
> + { return AVERROR_IO; }
wrong indention
> + vidbuf += rle_num_bytes;
> + bytes_copied += rle_num_bytes;
> + }
> + if(bytes_copied == vid->header.npixels)
> + {
> + // may contain a 0 byte even if read all pixels
> + if(get_byte(pb)) { url_fseek(pb, url_ftell(pb) - 1, SEEK_SET); }
SEEK_CUR
> + break;
> + }
> + if(bytes_copied > vid->header.npixels) { return -1; } // error
> + } while(rle_num_bytes);
> +
> + // copy packet data, and set some other parameters
> + vidbuf_nbytes = vidbuf - vidbuf_start; // doesn't undercount, increments to next byte above
> + if(av_new_packet(pkt, vidbuf_nbytes)) { return AVERROR_NOMEM; }
you cannot allocate the packet after setting its fields
[...]
> +static int vid_read_packet(AVFormatContext *s,
> + AVPacket *pkt)
> +{
> + BVID_DemuxContext *vid = (BVID_DemuxContext *)s->priv_data; // permanent data outside of function
> + ByteIOContext *pb = &s->pb; // io to file
> + unsigned char block_type; // block type
> + int audio_length;
> + int ret_value;
> +
> + if(vid->is_finished || url_feof(pb)) { return AVERROR_IO; }
> +
> + av_log(s, AV_LOG_VERBOSE, "[bethsoftvid demuxer]");
whats that good for?
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
I am the wisest man alive, for I know one thing, and that is that I know
nothing. -- Socrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070403/af0eec8b/attachment.pgp>
More information about the ffmpeg-devel
mailing list