[FFmpeg-devel] [PATCH]Addedd Kate demuxer and decoder

[Clément Bœsch]

On Tue, Oct 21, 2014 at 10:02:35PM +0530, Hima Dangi wrote:
> diff --git a/libavformat/Makefile b/libavformat/Makefile
> index 86064ea..4d41283 100644
> --- a/libavformat/Makefile
> +++ b/libavformat/Makefile

A git format-patch is prefered because it includes authorship, commit
message, commit description, ...

> @@ -433,6 +433,7 @@ OBJS-$(CONFIG_VOBSUB_DEMUXER)            += subtitles.o
> # mpeg demuxer is in the
>  OBJS-$(CONFIG_VOC_DEMUXER)               += vocdec.o voc.o
>  OBJS-$(CONFIG_VOC_MUXER)                 += vocenc.o voc.o
>  OBJS-$(CONFIG_VPLAYER_DEMUXER)           += vplayerdec.o subtitles.o
> +OBJS-$(CONFIG_KATE_DEMUXER)              += katedec.o subtitles.o
>  OBJS-$(CONFIG_VQF_DEMUXER)               += vqf.o

This is not the correct order

>  OBJS-$(CONFIG_W64_DEMUXER)               += wavdec.o w64.o pcm.o
>  OBJS-$(CONFIG_W64_MUXER)                 += wavenc.o w64.o
> diff --git a/libavformat/allformats.c b/libavformat/allformats.c
> index d54ed9b..6e16031 100644
> --- a/libavformat/allformats.c
> +++ b/libavformat/allformats.c
> @@ -320,7 +320,7 @@ void av_register_all(void)
>      REGISTER_DEMUXER (XWMA,             xwma);
>      REGISTER_DEMUXER (YOP,              yop);
>      REGISTER_MUXDEMUX(YUV4MPEGPIPE,     yuv4mpegpipe);
> -
> +    REGISTER_DEMUXER (KATE,             kate);

kate should be along its subtitles peers

>      /* image demuxers */
>      REGISTER_DEMUXER (IMAGE_BMP_PIPE,        image_bmp_pipe);
>      REGISTER_DEMUXER (IMAGE_DPX_PIPE,        image_dpx_pipe);
> diff --git a/libavformat/katedec.c b/libavformat/katedec.c
> new file mode 100644
> index 0000000..7b4ced3
> --- /dev/null
> +++ b/libavformat/katedec.c
> @@ -0,0 +1,158 @@

First, before I start. All the indentation in the file is completely
broken. See the other code source files for a reference, and
http://ffmpeg.org/developer.html#Coding-Rules-1

> +
> +
> +/**
> + * @file
> + * Kate subtitles format demuxer
> + */
> +
> +#include "avformat.h"
> +#include "internal.h"
> +#include "subtitles.h"
> +#include "libavutil/bprint.h"
> +#include "libavutil/intreadwrite.h"
> +
> +
> +typedef struct {
> +    FFDemuxSubtitlesQueue q;
> +} KateContext;
> +
> +static int kate_probe(AVProbeData *p)
> +{
> +    int v1,v2,v3,v4,v5,v6;
> +    const char *ptr = p->buf;
> +    const char *find;
> +    if(strstr(ptr,"kate {"))
> +    {
> +
> +      find=strstr(ptr,"event {");

What happens if you have "event" and the timestamp just after?

> +      while(strcmp(find,"\n")==0 || strcmp(find," ")==0)

This can not work, it was not tested.

Also, you can use a simple equality since it's only single characters.

> +       find++;
> +      if (  sscanf(find," %2d:%2d:%2d -->
> %2d:%2d:%2d",&v1,&v2,&v3,&v4,&v5,&v6 )>=4)

Why >= 4?

Also, your mail is mangled (lines are wrapped), you didn't send it
properly. Use git-send-email or configure your mailer.

> +      return AVPROBE_SCORE_MAX;
> +      else
> +      return 0;
> +     }
> +     else
> +     return 0;
> +}
> +
> +static int64_t get_pts_start(const char **buf)
> +{
> +
> +        int hh1, mm1, ss1;
> +        int hh2, mm2, ss2;
> +        const char *find = strstr(*buf,"event");
> +        while(strcmp(find,"\n")|| strcmp(find," "))
> +        find++;
> +        if(sscanf(find,  "%2d:%2d:%2d --> %2d:%2d:%2d",&hh1, &mm1,
> &ss1,&hh2, &mm2, &ss2)>=6)
> +        {
> +            int64_t start = (hh1*3600LL + mm1*60LL + ss1) * 1000LL;
> +            *buf += ff_subtitles_next_line(*buf);
> +            return start;
> +        }
> +       *buf += ff_subtitles_next_line(*buf);
> +      return AV_NOPTS_VALUE;
> +}
> +
> +static int64_t get_pts_end(const char **buf)
> +{

This function is almost identical to get_pts_start(), they should be
refactored.

> +
> +        int hh1, mm1, ss1;
> +        int hh2, mm2, ss2;
> +        const char *find = strstr(*buf,"event");
> +        while(strcmp(find,"\n") || strcmp(find," "))
> +        find++;
> +        if(sscanf(find,  "%2d:%2d:%2d --> %2d:%2d:%2d",&hh1, &mm1,
> &ss1,&hh2, &mm2, &ss2)>=6)
> +        {
> +            int64_t end = (hh2*3600LL + mm2*60LL + ss2) * 1000LL;
> 
> +            *buf += ff_subtitles_next_line(*buf);
> +            return end;
> +        }
> +       *buf += ff_subtitles_next_line(*buf);
> +      return AV_NOPTS_VALUE;
> +}
> +
> +static int kate_read_header(AVFormatContext *s)
> +{
> +    KateContext *kate = s->priv_data;
> +    AVStream *st = avformat_new_stream(s, NULL);
> +     int64_t pts_start;
> +        int64_t pts_end;
> +        int64_t duration;
> +    if (!st)
> +        return AVERROR(ENOMEM);
> +    avpriv_set_pts_info(st, 64, 1, 100);
> +    st->codec->codec_type = AVMEDIA_TYPE_SUBTITLE;
> +    st->codec->codec_id   = AV_CODEC_ID_TEXT;
> +
> +
> +
> +    while (!avio_feof(s->pb))
> +     {
> +        char line[4096];
> +        const char *p = line;
> +        const int64_t pos = avio_tell(s->pb);

> +        int len = ff_get_line(s->pb, line, sizeof(line));
> +

Since you're reading line per line, how can this work with multiline
events.

And typically, I'd be surprised if your code works with this:

kate {
  event
    {
       01:02:03 --> 04:05:06
             "foo bar"
    }
}

> +        AVPacket *sub;

> +
> +
> +

No need to add so much spacing

[...]

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20141021/481f7b23/attachment.asc>