[FFmpeg-devel] [PATCH] ffserver rtsp bug fixes

[Howard Chu]

Howard Chu wrote:
> Howard Chu wrote:
>> Howard Chu wrote:
>>> Howard Chu wrote:
>>>
>>>> Unfortunately I still haven't had any success actually playing the stream. It
>>>> seems that ffserver isn't sending any RTP packets at all.
>>>
>>> The cur_pts in the stream starts out with an absolute timestamp, while for all
>>> subsequent packets it only carries a relative timestamp. The check for whether
>>> it's time to transmit or not thus compares the huge cur_pts against the
>>> current server time (which is also relative) and decides it's not time to send
>>> anything yet.
>>>
>>> This patch gets things working for a single stream (audio only or video only).
>>> Still not working for both audio and video; in that case I only receive the audio.
>>
>> Finally found that problem too, it was a simple error in looking up the stream
>> index. The attached patch includes the two previous ones. I can now
>> successfully play ffserver's rtsp streams using both ffplayer and mplayer.
>> Unfortunately my G1 phone refuses to play the stream, so I still need to find
>> out what's wrong there.
>
> Again, it turned out to be something really simple - the sdp response was
> missing a required c= (connection) record. I've fixed this by using "0.0.0.0"
> when no IP address was provided by the calling app.
>
> Also re-sending the ffserver patch, slightly simplified. (When comparing the
> feed file to the current config, we only need to check that the codec_id
> matches, don't need to actually open the codec on each check.)
>
> So after spending a day and a half poking at this, I now have ffserver
> streaming live H264/AAC videos to my G1 phone via RTSP/RTP. I've also attached
> the ffserver.conf file I'm using. I invoke ffmpeg as
>
> ffmpeg -i<input>  -threads 0 http://localhost:8090/feed1.ffm

Martin suggested I repost the explanations of each patch, so I'm doing this here.

>
> Index: ffserver.c
> ===================================================================
> --- ffserver.c	(revision 22902)
> +++ ffserver.c	(working copy)
> @@ -2210,14 +2210,18 @@
>  static int64_t get_packet_send_clock(HTTPContext *c)
>  {
>      int bytes_left, bytes_sent, frame_bytes;
> +    int64_t pts = c->cur_pts;
>
> +    if (pts >= c->first_pts)
> +        pts -= c->first_pts;
> +
>      frame_bytes = c->cur_frame_bytes;
>      if (frame_bytes <= 0)
> -        return c->cur_pts;
> +        return pts;
>      else {
>          bytes_left = c->buffer_end - c->buffer_ptr;
>          bytes_sent = frame_bytes - bytes_left;
> -        return c->cur_pts + (c->cur_frame_duration * bytes_sent) / frame_bytes;
> +        return pts + (c->cur_frame_duration * bytes_sent) / frame_bytes;
>      }
>  }
>
When I was streaming an flv from ffmpeg to ffserver I saw that the first 
frames had absolute timestamps of the current real time. All subsequent 
packets only had relative timestamps. In http_send_data() a packet will only 
get sent if the packet timestamp is older than the current server time, but 
the server time is also checked as a relative timestamp. This check turned 
into (1234567890000 > 0) so no RTP packets were ever sent. This patch makes 
sure that only relative timestamps are returned from get_packet_send_clock.

> @@ -2342,7 +2346,7 @@
>                          }
>                      }
>                      for(i=0;i<c->stream->nb_streams;i++) {
> -                        if (c->feed_streams[i] == pkt.stream_index) {
> +                        if (c->stream->feed_streams[i] == pkt.stream_index) {
>                              AVStream *st = c->fmt_in->streams[source_index];
>                              pkt.stream_index = i;
>                              if (pkt.flags & AV_PKT_FLAG_KEY &&

With everything else fixed, streaming was only working for streams with a 
single track - audio-only or video-only. The problem in this test is that 
c->feed_streams[] is always zero-filled because c is a single RTP stream and 
RTP streams only ever carry one channel. You need to use c->stream which 
actually carries the information about all of the related channels for the 
overall stream. This patch gets audio+video working.

> @@ -3679,7 +3683,7 @@
>                              ccs = ss->codec;
>  #define CHECK_CODEC(x)  (ccf->x != ccs->x)
>
> -                            if (CHECK_CODEC(codec) || CHECK_CODEC(codec_type)) {
> +                            if (CHECK_CODEC(codec_id) || CHECK_CODEC(codec_type)) {
>                                  http_log("Codecs do not match for stream %d\n", i);
>                                  matches = 0;
>                              } else if (CHECK_CODEC(bit_rate) || CHECK_CODEC(flags)) {

The check here is comparing a stream that was just opened with 
av_open_input_file(). That function doesn't initialize any codecs, so 
ccs->codec is always NULL. Checking the codec_id is the correct step.

> @@ -3803,6 +3807,7 @@
>  static void add_codec(FFStream *stream, AVCodecContext *av)
>  {
>      AVStream *st;
> +    AVCodec *codec;
>
>      /* compute default parameters */
>      switch(av->codec_type) {
> @@ -3842,7 +3847,8 @@
>              av->nsse_weight = 8;
>
>          av->frame_skip_cmp = FF_CMP_DCTMAX;
> -        av->me_method = ME_EPZS;
> +        if (!av->me_method)
> +            av->me_method = ME_EPZS;
>          av->rc_buffer_aggressivity = 1.0;
>
>          if (!av->rc_eq)

There are a lot of unconditional option settings here. I'm not sure why 
they're being done, since these options can be set using AVOptionVideo in 
ffserver.conf. This fixes just one case of add_codec overriding what was set 
in the config file; there are others too but I don't know how important any of 
it is overall.

> @@ -3872,6 +3878,15 @@
>      st->codec = avcodec_alloc_context();
>      stream->streams[stream->nb_streams++] = st;
>      memcpy(st->codec, av, sizeof(AVCodecContext));
> +    codec = avcodec_find_encoder(st->codec->codec_id);
> +    if (!codec) {
> +        http_log("Encoder (codec id %d) not found\n", st->codec->codec_id);
> +        exit(1);
> +    }
> +    if (avcodec_open(st->codec, codec) < 0) {
> +        http_log("Error opening encoder\n");
> +        exit(1);
> +    }
>  }
>
>  static enum CodecID opt_audio_codec(const char *arg)

This was the first fix I posted. The codec must actually be opened/initialized 
to give it a chance to set up its extradata when global headers are 
configured. Otherwise the config option has no effect. Without this patch RTSP 
always fails when trying to use AAC; you get the message "AAC with no global 
headers is currently not supported."

-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/