[FFmpeg-devel] [PATCH 1/3] avformat/electronicarts: fix EOF check

[FFmpeg-devel] [PATCH 1/3] avformat/electronicarts: fix EOF check

[Marton Balint]

Similar to feof(), avio_feof() is only true after an actual overread.

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavformat/electronicarts.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/electronicarts.c b/libavformat/electronicarts.c
index e7f0cb9b9e..0a0d6249e8 100644
--- a/libavformat/electronicarts.c
+++ b/libavformat/electronicarts.c
@@ -582,9 +582,9 @@ static int ea_read_packet(AVFormatContext *s, AVPacket *pkt)
     int av_uninit(num_samples);
 
     while ((!packet_read && !hit_end) || partial_packet) {
+        chunk_type = avio_rl32(pb);
         if (avio_feof(pb))
             return AVERROR_EOF;
-        chunk_type = avio_rl32(pb);
         chunk_size = ea->big_endian ? avio_rb32(pb) : avio_rl32(pb);
         if (chunk_size < 8)
             return AVERROR_INVALIDDATA;
-- 
2.35.3

_______________________________________________
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".

[FFmpeg-devel] [PATCH 2/3] avformat/electronicarts: set packet_read in one place

[Marton Balint]

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavformat/electronicarts.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavformat/electronicarts.c b/libavformat/electronicarts.c
index 0a0d6249e8..0532264f38 100644
--- a/libavformat/electronicarts.c
+++ b/libavformat/electronicarts.c
@@ -726,8 +726,9 @@ get_video_packet:
                 ret = av_append_packet(pb, pkt, chunk_size);
             } else
                 ret = av_get_packet(pb, pkt, chunk_size);
+            packet_read = 1;
+
             if (ret < 0) {
-                packet_read = 1;
                 partial_packet = 0;
                 break;
             }
@@ -737,7 +738,6 @@ get_video_packet:
             else
                 pkt->stream_index = ea->video.stream_index;
             pkt->flags       |= key;
-            packet_read       = 1;
             break;
 
         default:
-- 
2.35.3

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 2/3] avformat/electronicarts: set packet_read in one place

[Peter Ross]

[-- Attachment #1.1: Type: text/plain, Size: 1118 bytes --]

On Sun, Nov 13, 2022 at 07:44:40PM +0100, Marton Balint wrote:
> Signed-off-by: Marton Balint <cus@passwd.hu>
> ---
>  libavformat/electronicarts.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libavformat/electronicarts.c b/libavformat/electronicarts.c
> index 0a0d6249e8..0532264f38 100644
> --- a/libavformat/electronicarts.c
> +++ b/libavformat/electronicarts.c
> @@ -726,8 +726,9 @@ get_video_packet:
>                  ret = av_append_packet(pb, pkt, chunk_size);
>              } else
>                  ret = av_get_packet(pb, pkt, chunk_size);
> +            packet_read = 1;
> +
>              if (ret < 0) {
> -                packet_read = 1;
>                  partial_packet = 0;
>                  break;
>              }
> @@ -737,7 +738,6 @@ get_video_packet:
>              else
>                  pkt->stream_index = ea->video.stream_index;
>              pkt->flags       |= key;
> -            packet_read       = 1;
>              break;
>  
>          default:

lgtm

-- Peter
(A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B)

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

[FFmpeg-devel] [PATCH 3/3] avformat/electronicarts: add option to return alpha channel in the main video stream in VP6A codec

[Marton Balint]

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 doc/demuxers.texi            | 18 ++++++++++++++++
 libavformat/electronicarts.c | 42 +++++++++++++++++++++++++++++++-----
 libavformat/version.h        |  2 +-
 3 files changed, 56 insertions(+), 6 deletions(-)

diff --git a/doc/demuxers.texi b/doc/demuxers.texi
index 2b6dd86c2a..f07f3f5318 100644
--- a/doc/demuxers.texi
+++ b/doc/demuxers.texi
@@ -285,6 +285,24 @@ This demuxer accepts the following option:
 
 @end table
 
+@section ea
+
+Electronic Arts Multimedia format demuxer.
+
+This format is used by various Electronic Arts games.
+
+@subsection Options
+
+@table @option
+
+@item merge_alpha @var{bool}
+
+Normally the VP6 alpha channel (if exists) is returned as a secondary video
+stream, by setting this option you can make the demuxer return a single video
+stream which contains the alpha channel in addition to the ordinary video.
+
+@end table
+
 @section imf
 
 Interoperable Master Format demuxer.
diff --git a/libavformat/electronicarts.c b/libavformat/electronicarts.c
index 0532264f38..e7f574aede 100644
--- a/libavformat/electronicarts.c
+++ b/libavformat/electronicarts.c
@@ -28,6 +28,7 @@
 #include <inttypes.h>
 
 #include "libavutil/intreadwrite.h"
+#include "libavutil/opt.h"
 #include "avformat.h"
 #include "internal.h"
 
@@ -75,6 +76,8 @@ typedef struct VideoProperties {
 } VideoProperties;
 
 typedef struct EaDemuxContext {
+    const AVClass *class;
+
     int big_endian;
 
     VideoProperties video, alpha;
@@ -88,6 +91,7 @@ typedef struct EaDemuxContext {
     int num_samples;
 
     int platform;
+    int merge_alpha;
 } EaDemuxContext;
 
 static uint32_t read_arbitrary(AVIOContext *pb)
@@ -442,6 +446,10 @@ static int process_ea_header(AVFormatContext *s)
 
         case AVhd_TAG:
             err = process_video_header_vp6(s, &ea->alpha);
+            if (err >= 0 && ea->video.codec == AV_CODEC_ID_VP6 && ea->merge_alpha) {
+                ea->alpha.codec = 0;
+                ea->video.codec = AV_CODEC_ID_VP6A;
+            }
             break;
         }
 
@@ -578,7 +586,7 @@ static int ea_read_packet(AVFormatContext *s, AVPacket *pkt)
     int partial_packet = 0;
     int hit_end = 0;
     unsigned int chunk_type, chunk_size;
-    int ret = 0, packet_read = 0, key = 0;
+    int ret = 0, packet_read = 0, key = 0, vp6a;
     int av_uninit(num_samples);
 
     while ((!packet_read && !hit_end) || partial_packet) {
@@ -721,19 +729,28 @@ static int ea_read_packet(AVFormatContext *s, AVPacket *pkt)
 get_video_packet:
             if (!chunk_size)
                 continue;
+            if (chunk_size > INT_MAX - 3)
+                return AVERROR_INVALIDDATA;
+
+            vp6a = (ea->video.codec == AV_CODEC_ID_VP6A && (chunk_type == MV0F_TAG || chunk_type == MV0K_TAG));
 
             if (partial_packet) {
                 ret = av_append_packet(pb, pkt, chunk_size);
-            } else
-                ret = av_get_packet(pb, pkt, chunk_size);
+            } else {
+                if (vp6a)
+                    avio_seek(pb, -3, SEEK_CUR);
+                ret = av_get_packet(pb, pkt, chunk_size + (vp6a ? 3 : 0));
+                if (ret >= 0 && vp6a)
+                   AV_WB24(pkt->data, chunk_size);
+            }
             packet_read = 1;
 
             if (ret < 0) {
                 partial_packet = 0;
                 break;
             }
-            partial_packet = chunk_type == MVIh_TAG;
-            if (chunk_type == AV0K_TAG || chunk_type == AV0F_TAG)
+            partial_packet = vp6a || chunk_type == MVIh_TAG;
+            if (ea->alpha.codec && (chunk_type == AV0K_TAG || chunk_type == AV0F_TAG))
                 pkt->stream_index = ea->alpha.stream_index;
             else
                 pkt->stream_index = ea->video.stream_index;
@@ -752,6 +769,20 @@ get_video_packet:
     return ret;
 }
 
+#define OFFSET(x) offsetof(EaDemuxContext, x)
+#define FLAGS AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_DECODING_PARAM
+static const AVOption options[] = {
+    {"merge_alpha", "return VP6 alpha in the main video stream", OFFSET(merge_alpha), AV_OPT_TYPE_BOOL,  {.i64 = 0}, 0, 1, FLAGS },
+    {NULL}
+};
+
+static const AVClass ea_class = {
+    .class_name = "ea demuxer",
+    .item_name  = av_default_item_name,
+    .option     = options,
+    .version    = LIBAVUTIL_VERSION_INT,
+};
+
 const AVInputFormat ff_ea_demuxer = {
     .name           = "ea",
     .long_name      = NULL_IF_CONFIG_SMALL("Electronic Arts Multimedia"),
@@ -759,4 +790,5 @@ const AVInputFormat ff_ea_demuxer = {
     .read_probe     = ea_probe,
     .read_header    = ea_read_header,
     .read_packet    = ea_read_packet,
+    .priv_class     = &ea_class,
 };
diff --git a/libavformat/version.h b/libavformat/version.h
index 7c9d50b7b3..a7e5a9ac66 100644
--- a/libavformat/version.h
+++ b/libavformat/version.h
@@ -32,7 +32,7 @@
 #include "version_major.h"
 
 #define LIBAVFORMAT_VERSION_MINOR  34
-#define LIBAVFORMAT_VERSION_MICRO 101
+#define LIBAVFORMAT_VERSION_MICRO 102
 
 #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
                                                LIBAVFORMAT_VERSION_MINOR, \
-- 
2.35.3

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 3/3] avformat/electronicarts: add option to return alpha channel in the main video stream in VP6A codec

[Anton Khirnov]

Quoting Marton Balint (2022-11-13 19:44:41)
> Signed-off-by: Marton Balint <cus@passwd.hu>
> ---
>  doc/demuxers.texi            | 18 ++++++++++++++++
>  libavformat/electronicarts.c | 42 +++++++++++++++++++++++++++++++-----
>  libavformat/version.h        |  2 +-
>  3 files changed, 56 insertions(+), 6 deletions(-)
> 
> diff --git a/doc/demuxers.texi b/doc/demuxers.texi
> index 2b6dd86c2a..f07f3f5318 100644
> --- a/doc/demuxers.texi
> +++ b/doc/demuxers.texi
> @@ -285,6 +285,24 @@ This demuxer accepts the following option:
>  
>  @end table
>  
> +@section ea
> +
> +Electronic Arts Multimedia format demuxer.
> +
> +This format is used by various Electronic Arts games.
> +
> +@subsection Options
> +
> +@table @option
> +
> +@item merge_alpha @var{bool}
> +
> +Normally the VP6 alpha channel (if exists) is returned as a secondary video
> +stream,

Why? And why keep it as the default?

-- 
Anton Khirnov
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 3/3] avformat/electronicarts: add option to return alpha channel in the main video stream in VP6A codec

[Marton Balint]

On Mon, 14 Nov 2022, Anton Khirnov wrote:

> Quoting Marton Balint (2022-11-13 19:44:41)
>> Signed-off-by: Marton Balint <cus@passwd.hu>
>> ---
>>  doc/demuxers.texi            | 18 ++++++++++++++++
>>  libavformat/electronicarts.c | 42 +++++++++++++++++++++++++++++++-----
>>  libavformat/version.h        |  2 +-
>>  3 files changed, 56 insertions(+), 6 deletions(-)
>>
>> diff --git a/doc/demuxers.texi b/doc/demuxers.texi
>> index 2b6dd86c2a..f07f3f5318 100644
>> --- a/doc/demuxers.texi
>> +++ b/doc/demuxers.texi
>> @@ -285,6 +285,24 @@ This demuxer accepts the following option:
>>
>>  @end table
>>
>> +@section ea
>> +
>> +Electronic Arts Multimedia format demuxer.
>> +
>> +This format is used by various Electronic Arts games.
>> +
>> +@subsection Options
>> +
>> +@table @option
>> +
>> +@item merge_alpha @var{bool}
>> +
>> +Normally the VP6 alpha channel (if exists) is returned as a secondary video
>> +stream,
>
> Why? And why keep it as the default?

VP6 alpha in EA format is a second VP6 encoded video stream where only the 
Y component is used and is interpreted as the alpha channel of the first 
VP6 stream. The alpha VP6 stream is muxed separately from the main VP6 
stream, has its own stream headers and packet headers. In theory the two 
streams might not even have the same resolution (although most likely 
that is not something that is seen or supported in the wild), but the 
format is capable of doing it.

Merged VP6 alpha (also known as the VP6A codec) means that a packet of the 
video stream contains the corresponding packet of both VP6 substreams like 
this:
{OffsetOfAlpha, DataPacket, AlphaDataPacet}
So data and alpha data of a frame is merged to a single packet, this is 
how VP6 video with alpha is muxed in FLV and SWF.

So the first approach is more like how the demuxer sees data in the EA 
format, unfortunately it is different to what the FLV or SWF format 
expects, so - having no better place for it in the framework - I decided 
to do an optional format conversion in the EA demuxer.

I did not want to change the default, but certainly doable if people 
prefer it.

Regards,
Marton
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 3/3] avformat/electronicarts: add option to return alpha channel in the main video stream in VP6A codec

[Peter Ross]

[-- Attachment #1.1: Type: text/plain, Size: 2481 bytes --]

On Mon, Nov 14, 2022 at 11:05:44PM +0100, Marton Balint wrote:
> 
> 
> On Mon, 14 Nov 2022, Anton Khirnov wrote:
> 
> > Quoting Marton Balint (2022-11-13 19:44:41)
> > > Signed-off-by: Marton Balint <cus@passwd.hu>
> > > ---
> > >  doc/demuxers.texi            | 18 ++++++++++++++++
> > >  libavformat/electronicarts.c | 42 +++++++++++++++++++++++++++++++-----
> > >  libavformat/version.h        |  2 +-
> > >  3 files changed, 56 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/doc/demuxers.texi b/doc/demuxers.texi
> > > index 2b6dd86c2a..f07f3f5318 100644
> > > --- a/doc/demuxers.texi
> > > +++ b/doc/demuxers.texi
> > > @@ -285,6 +285,24 @@ This demuxer accepts the following option:
> > > 
> > >  @end table
> > > 
> > > +@section ea
> > > +
> > > +Electronic Arts Multimedia format demuxer.
> > > +
> > > +This format is used by various Electronic Arts games.
> > > +
> > > +@subsection Options
> > > +
> > > +@table @option
> > > +
> > > +@item merge_alpha @var{bool}
> > > +
> > > +Normally the VP6 alpha channel (if exists) is returned as a secondary video
> > > +stream,
> > 
> > Why? And why keep it as the default?
> 
> VP6 alpha in EA format is a second VP6 encoded video stream where only the Y
> component is used and is interpreted as the alpha channel of the first VP6
> stream. The alpha VP6 stream is muxed separately from the main VP6 stream,
> has its own stream headers and packet headers. In theory the two streams
> might not even have the same resolution (although most likely that is not
> something that is seen or supported in the wild), but the format is capable
> of doing it.
> 
> Merged VP6 alpha (also known as the VP6A codec) means that a packet of the
> video stream contains the corresponding packet of both VP6 substreams like
> this:
> {OffsetOfAlpha, DataPacket, AlphaDataPacet}
> So data and alpha data of a frame is merged to a single packet, this is how
> VP6 video with alpha is muxed in FLV and SWF.
> 
> So the first approach is more like how the demuxer sees data in the EA
> format, unfortunately it is different to what the FLV or SWF format expects,
> so - having no better place for it in the framework - I decided to do an
> optional format conversion in the EA demuxer.
> 
> I did not want to change the default, but certainly doable if people prefer
> it.

looks good. i have no preference.

-- Peter
(A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B)

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 3/3] avformat/electronicarts: add option to return alpha channel in the main video stream in VP6A codec

[Marton Balint]

On Thu, 17 Nov 2022, Peter Ross wrote:

> On Mon, Nov 14, 2022 at 11:05:44PM +0100, Marton Balint wrote:
>>
>>
>> On Mon, 14 Nov 2022, Anton Khirnov wrote:
>>
>>> Quoting Marton Balint (2022-11-13 19:44:41)
>>>> Signed-off-by: Marton Balint <cus@passwd.hu>
>>>> ---
>>>>  doc/demuxers.texi            | 18 ++++++++++++++++
>>>>  libavformat/electronicarts.c | 42 +++++++++++++++++++++++++++++++-----
>>>>  libavformat/version.h        |  2 +-
>>>>  3 files changed, 56 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/doc/demuxers.texi b/doc/demuxers.texi
>>>> index 2b6dd86c2a..f07f3f5318 100644
>>>> --- a/doc/demuxers.texi
>>>> +++ b/doc/demuxers.texi
>>>> @@ -285,6 +285,24 @@ This demuxer accepts the following option:
>>>>
>>>>  @end table
>>>>
>>>> +@section ea
>>>> +
>>>> +Electronic Arts Multimedia format demuxer.
>>>> +
>>>> +This format is used by various Electronic Arts games.
>>>> +
>>>> +@subsection Options
>>>> +
>>>> +@table @option
>>>> +
>>>> +@item merge_alpha @var{bool}
>>>> +
>>>> +Normally the VP6 alpha channel (if exists) is returned as a secondary video
>>>> +stream,
>>>
>>> Why? And why keep it as the default?
>>
>> VP6 alpha in EA format is a second VP6 encoded video stream where only the Y
>> component is used and is interpreted as the alpha channel of the first VP6
>> stream. The alpha VP6 stream is muxed separately from the main VP6 stream,
>> has its own stream headers and packet headers. In theory the two streams
>> might not even have the same resolution (although most likely that is not
>> something that is seen or supported in the wild), but the format is capable
>> of doing it.
>>
>> Merged VP6 alpha (also known as the VP6A codec) means that a packet of the
>> video stream contains the corresponding packet of both VP6 substreams like
>> this:
>> {OffsetOfAlpha, DataPacket, AlphaDataPacet}
>> So data and alpha data of a frame is merged to a single packet, this is how
>> VP6 video with alpha is muxed in FLV and SWF.
>>
>> So the first approach is more like how the demuxer sees data in the EA
>> format, unfortunately it is different to what the FLV or SWF format expects,
>> so - having no better place for it in the framework - I decided to do an
>> optional format conversion in the EA demuxer.
>>
>> I did not want to change the default, but certainly doable if people prefer
>> it.
>
> looks good. i have no preference.

Thanks, applied the series.

Regards,
Marton
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 1/3] avformat/electronicarts: fix EOF check

[Peter Ross]

[-- Attachment #1.1: Type: text/plain, Size: 1055 bytes --]

On Sun, Nov 13, 2022 at 07:44:39PM +0100, Marton Balint wrote:
> Similar to feof(), avio_feof() is only true after an actual overread.
> 
> Signed-off-by: Marton Balint <cus@passwd.hu>
> ---
>  libavformat/electronicarts.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavformat/electronicarts.c b/libavformat/electronicarts.c
> index e7f0cb9b9e..0a0d6249e8 100644
> --- a/libavformat/electronicarts.c
> +++ b/libavformat/electronicarts.c
> @@ -582,9 +582,9 @@ static int ea_read_packet(AVFormatContext *s, AVPacket *pkt)
>      int av_uninit(num_samples);
>  
>      while ((!packet_read && !hit_end) || partial_packet) {
> +        chunk_type = avio_rl32(pb);
>          if (avio_feof(pb))
>              return AVERROR_EOF;
> -        chunk_type = avio_rl32(pb);
>          chunk_size = ea->big_endian ? avio_rb32(pb) : avio_rl32(pb);
>          if (chunk_size < 8)
>              return AVERROR_INVALIDDATA;
> -- 
> 2.35.3

lgtm

-- Peter
(A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B)

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".