diff mbox

[FFmpeg-devel,v5] lavf/flv: Add XV (Xunlei Video) Support. Fixes ticket #3720

Message ID b64dd77c-eae2-34e3-994e-979e058fa256@iitk.ac.in
State Superseded
Headers show

Commit Message

Shivam Goyal April 9, 2019, 2:19 p.m. UTC
lavf/flv: Add XV (Xunlei Video) Support.

Fixes ticket #3720

I have fixed the minor mistakes in the patch. I have also checked the 
patch with patcheck.

Comments

Carl Eugen Hoyos April 9, 2019, 2:31 p.m. UTC | #1
2019-04-09 16:19 GMT+02:00, Shivam Goyal <shivgo@iitk.ac.in>:
> lavf/flv: Add XV (Xunlei Video) Support.
>
> Fixes ticket #3720
>
> I have fixed the minor mistakes in the patch. I have also
> checked the patch with patcheck.

> +    offset = ((avio_r8(ic) + rot & 0xff) << 24 |
> +                (avio_r8(ic) + rot & 0xff) << 16 |
> +                (avio_r8(ic) + rot & 0xff) << 8 |
> +                (avio_r8(ic) + rot & 0xff)) + 0x200000;

Should still be:
    offset = ((avio_r8(ic) + rot & 0xff) << 24 |
              (avio_r8(ic) + rot & 0xff) << 16 |
              (avio_r8(ic) + rot & 0xff) <<  8 |
              (avio_r8(ic) + rot & 0xff)) + 0x200000;
There is no 8/12-char indentation, only vertical alignment.

Please bump libavformat minor version.

Carl Eugen
Moritz Barsnick April 9, 2019, 3:02 p.m. UTC | #2
Some minor nitpicking remarks from me:

> Subject: [PATCH] lavf/flv: Add XV (Xunlei Video) Support. Fixes ticket #3720.

Basically, the second sentence belongs into the body of the commit
message, after a single empty line separator. (It may not matter here.)

>  Changelog                |  1 +
>  libavformat/Makefile     |  1 +
>  libavformat/allformats.c |  1 +
>  libavformat/flvdec.c     | 85 ++++++++++++++++++++++++++++++++++++++++

You could also add a documentation entry for xv, referring to the flv
doc. That would be in doc/demuxers.texi. (Can be added later, IMO.)

> +    if (d[0] == 'X' &&
> +        d[1] == 'L' &&
> +        d[2] == 'V' &&
> +        d[3] == 'F') {

Question to the other core developers: Is there any preference for this
style of code over:
       if (AV_RL32(d) == MKTAG('X', 'L', 'V', 'F')
which some other probes use?
(flv rightly doesn't use the latter style, because d[3] is variable.)

> +        return AVPROBE_SCORE_EXTENSION + 1;

I believe Carl Eugen asked you to change this to:
AVPROBE_SCORE_EXTENSION / 2 + 1

> +    if (!strcmp(s->iformat->name , "xv")) {
> +        for (i=0; i < FFMIN(2,fileposlen); i++) {
                i = 0 (whitespace around all operators)
> +};
> \ No newline at end of file

You should also consider adding a newline at the end of the last line.

If this works (I haven't tested): Nice improvement versus your first
patch!

Cheers,
Moritz
Shivam Goyal April 9, 2019, 3:15 p.m. UTC | #3
On 09-04-2019 08:32 PM, Moritz Barsnick wrote:
> Some minor nitpicking remarks from me:
>
>> Subject: [PATCH] lavf/flv: Add XV (Xunlei Video) Support. Fixes ticket #3720.
> Basically, the second sentence belongs into the body of the commit
> message, after a single empty line separator. (It may not matter here.)
>
>>   Changelog                |  1 +
>>   libavformat/Makefile     |  1 +
>>   libavformat/allformats.c |  1 +
>>   libavformat/flvdec.c     | 85 ++++++++++++++++++++++++++++++++++++++++
> You could also add a documentation entry for xv, referring to the flv
> doc. That would be in doc/demuxers.texi. (Can be added later, IMO.)
Would add documentation entry too.
>
>> +    if (d[0] == 'X' &&
>> +        d[1] == 'L' &&
>> +        d[2] == 'V' &&
>> +        d[3] == 'F') {
> Question to the other core developers: Is there any preference for this
> style of code over:
>         if (AV_RL32(d) == MKTAG('X', 'L', 'V', 'F')
> which some other probes use?
> (flv rightly doesn't use the latter style, because d[3] is variable.)
>
>> +        return AVPROBE_SCORE_EXTENSION + 1;
> I believe Carl Eugen asked you to change this to:
> AVPROBE_SCORE_EXTENSION / 2 + 1

Carl Eugen suggested me to change to AVPROBE_SCORE_MAX /2 +1

AVPROBE_SCORE_MAX has a value of 100.

But the AVPROBE_SCORE_EXTENSION already has a value of 50. so, I change 
it to that

>> +    if (!strcmp(s->iformat->name , "xv")) {
>> +        for (i=0; i < FFMIN(2,fileposlen); i++) {
>                  i = 0 (whitespace around all operators)
>> +};
>> \ No newline at end of file
> You should also consider adding a newline at the end of the last line.
>
> If this works (I haven't tested): Nice improvement versus your first
> patch!
Thanks, I will add the new line too.
> Cheers,
> Moritz
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Moritz Barsnick April 9, 2019, 5:39 p.m. UTC | #4
On Tue, Apr 09, 2019 at 20:45:41 +0530, Shivam Goyal wrote:
> > I believe Carl Eugen asked you to change this to:
> > AVPROBE_SCORE_EXTENSION / 2 + 1
> Carl Eugen suggested me to change to AVPROBE_SCORE_MAX /2 +1

Ah, sorry, my bad - I missed that.

Thanks,
Moritz
diff mbox

Patch

From a64698a11c10ec62968a5d853d7db0fed956c0d3 Mon Sep 17 00:00:00 2001
From: Shivam Goyal <shivgo@iitk.ac.in>
Date: Tue, 9 Apr 2019 19:25:23 +0530
Subject: [PATCH] lavf/flv: Add XV (Xunlei Video) Support. Fixes ticket #3720.

---
 Changelog                |  1 +
 libavformat/Makefile     |  1 +
 libavformat/allformats.c |  1 +
 libavformat/flvdec.c     | 85 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 88 insertions(+)

diff --git a/Changelog b/Changelog
index 8c866cd0c2..4285c20f34 100644
--- a/Changelog
+++ b/Changelog
@@ -22,6 +22,7 @@  version <next>:
 - removed libndi-newtek
 - agm decoder
 - KUX demuxer
+- XV (Xunlie Video) demuxer
 
 
 version 4.1:
diff --git a/libavformat/Makefile b/libavformat/Makefile
index 99be60d184..e090c051f1 100644
--- a/libavformat/Makefile
+++ b/libavformat/Makefile
@@ -561,6 +561,7 @@  OBJS-$(CONFIG_WV_MUXER)                  += wvenc.o wv.o apetag.o img2.o
 OBJS-$(CONFIG_XA_DEMUXER)                += xa.o
 OBJS-$(CONFIG_XBIN_DEMUXER)              += bintext.o sauce.o
 OBJS-$(CONFIG_XMV_DEMUXER)               += xmv.o
+OBJS-$(CONFIG_XV_DEMUXER)                += flvdec.o
 OBJS-$(CONFIG_XVAG_DEMUXER)              += xvag.o
 OBJS-$(CONFIG_XWMA_DEMUXER)              += xwma.o
 OBJS-$(CONFIG_YOP_DEMUXER)               += yop.o
diff --git a/libavformat/allformats.c b/libavformat/allformats.c
index d316a0529a..b499186071 100644
--- a/libavformat/allformats.c
+++ b/libavformat/allformats.c
@@ -456,6 +456,7 @@  extern AVOutputFormat ff_wv_muxer;
 extern AVInputFormat  ff_xa_demuxer;
 extern AVInputFormat  ff_xbin_demuxer;
 extern AVInputFormat  ff_xmv_demuxer;
+extern AVInputFormat  ff_xv_demuxer;
 extern AVInputFormat  ff_xvag_demuxer;
 extern AVInputFormat  ff_xwma_demuxer;
 extern AVInputFormat  ff_yop_demuxer;
diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c
index b531a39adc..8226988834 100644
--- a/libavformat/flvdec.c
+++ b/libavformat/flvdec.c
@@ -127,6 +127,19 @@  static int kux_probe(const AVProbeData *p)
     return 0;
 }
 
+static int xv_probe(const AVProbeData *p)
+{
+    const uint8_t *d = p->buf;
+
+    if (d[0] == 'X' &&
+        d[1] == 'L' &&
+        d[2] == 'V' &&
+        d[3] == 'F') {
+        return AVPROBE_SCORE_EXTENSION + 1;
+    }
+    return 0;
+}
+
 static void add_keyframes_index(AVFormatContext *s)
 {
     FLVContext *flv   = s->priv_data;
@@ -459,6 +472,12 @@  static int parse_keyframes_index(AVFormatContext *s, AVIOContext *ioc, int64_t m
         }
     }
 
+    if (!strcmp(s->iformat->name , "xv")) {
+        for (i=0; i < FFMIN(2,fileposlen); i++) {
+            filepositions[i] += 0x200000;
+        }
+    }
+
     if (timeslen == fileposlen && fileposlen>1 && max_pos <= filepositions[0]) {
         for (i = 0; i < FFMIN(2,fileposlen); i++) {
             flv->validate_index[i].pos = filepositions[i];
@@ -783,6 +802,51 @@  static int flv_read_header(AVFormatContext *s)
     return 0;
 }
 
+static int xv_read_header(AVFormatContext *s)
+{
+    int flags;
+    FLVContext *xv = s->priv_data;
+    AVIOContext *ic = s->pb;
+    int offset;
+    int rot;
+    int64_t pos;
+
+    //Find the rot value for rotating the bytes
+    avio_skip(ic, 0x200000);
+    rot = 0x46 - avio_r8(ic);
+
+    avio_skip(ic, 3);
+
+    flags = (avio_r8(ic) + rot) & 0xff;
+
+    xv->missing_streams = flags & (FLV_HEADER_FLAG_HASVIDEO | FLV_HEADER_FLAG_HASAUDIO);
+
+    s->ctx_flags |= AVFMTCTX_NOHEADER;
+
+    offset = ((avio_r8(ic) + rot & 0xff) << 24 |
+                (avio_r8(ic) + rot & 0xff) << 16 |
+                (avio_r8(ic) + rot & 0xff) << 8 |
+                (avio_r8(ic) + rot & 0xff)) + 0x200000;
+
+    avio_seek(ic, offset + 4, SEEK_SET);
+
+
+    // Will modify the current buffer, as only
+    // the bytes from 0x200000 to 0x200400 are needed to decode
+    pos = ic->pos + ic->buf_ptr - ic->buf_end;
+    for (int i = 0; i < 0x400; i++) {
+        if (pos >= 0x200400) break;
+        ic->buf_ptr[i] = (ic->buf_ptr[i] + rot) & 0xff;
+        pos++;
+    }
+
+    s->start_time = 0;
+    xv->sum_flv_tag_size = 0;
+    xv->last_keyframe_stream_index = -1;
+
+    return 0;
+}
+
 static int flv_read_close(AVFormatContext *s)
 {
     int i;
@@ -1424,3 +1488,24 @@  AVInputFormat ff_kux_demuxer = {
     .extensions     = "kux",
     .priv_class     = &kux_class,
 };
+
+static const AVClass xv_class = {
+    .class_name = "xvdec",
+    .item_name  = av_default_item_name,
+    .option     = options,
+    .version    = LIBAVUTIL_VERSION_INT,
+};
+
+AVInputFormat ff_xv_demuxer = {
+    .name           = "xv",
+    .long_name      = NULL_IF_CONFIG_SMALL("Xunlei(Thunder) Video File"),
+    .priv_data_size = sizeof(FLVContext),
+    .read_probe     = xv_probe,
+    .read_header    = xv_read_header,
+    .read_packet    = flv_read_packet,
+    .read_seek      = flv_read_seek,
+    .read_close     = flv_read_close,
+    .extensions     = "xv",
+    .priv_class     = &xv_class,
+    .flags          = AVFMT_TS_DISCONT
+};
\ No newline at end of file
-- 
2.21.0