[FFmpeg-devel] [PATCH] RTSP-MS 15/15: move packet_time_start zero value assignment in asf.c

[Ronald S. Bultje]

Hi Michael,

On Fri, Apr 3, 2009 at 10:53 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
> what does this patch do and why?
> and [1] points to something random
> (yeah ive said in the past that patches and bugs refering to external
> ?discussion while ommiting a basic 2 sentance description are always
> ?rejected, this one here is a good example why this is very bad
> ?practice)

The ASF demuxer works as follows (pseudo-code):

read_packet(pkt) {
  while (1) {
    if (decode_packet(pkt))
      break;
    if (!sync_on_packet_header())
      assert(some condition);
    // that one line sits here
  }
  return 0;
}

sync_on_packet() syncs on an ASF packet, which might contain 0-N
"useful parsed data packets" (e.g. multiple video frames, or a video
frame divided over multiple ASF packets, same for audio). When we have
a complete data unit, it returns.

The RTP/ASF code does something similar with some slight changes
because of exact 1 RTP packet == 1 ASF packet mapping. However, we
don't use read_packet(), but instead directly use the decode_packet()
and sync_on_packet_header() functions so that this one line that
resets something in the ASF header is not called, and thus the ASF
demuxer state is never reset. This patch moves it to the place where
all other conditions are reset (in sync_on_packet_header()) so that
ASF works over RTSP as well.

You've asked in the past why it was needed: so far it wasn't, but now
it's needed for RTSP. You'll probably next ask why it wasn't there to
begin with, and my best bet for that is that the ASF demuxer is hacked
together rather than intelligently designed (take that, evolutionists!
:-) ). If you had reviewed whoever added this hack in it, you would've
told them to move it to a place where all other variables were reset
for the next packet as well. My patch does exactly that (and makes
RTSP/ASF work).

Ronald