libquicktime-devel — Discussion about development

Re: [Libquicktime-devel] [PATCH] ProRes support

[Maksalov B. <bor...@ya...>]

Updated ProRes patch attached.

Re: [Libquicktime-devel] [PATCH] ProRes support

[Burkhard P. <pl...@ip...>]

Hi,

Am 07.08.2012 10:28, schrieb Maksalov Boris:
>> Up to now I completely ignored libav. Are there any differences in version numbers?
>> If yes, that would be very bad.
> Well, the ProRes encoder first appeared in ffmpeg at:
> LIBAVCODEC_VERSION_MAJOR 53
> LIBAVCODEC_VERSION_MINOR 24
> LIBAVCODEC_VERSION_MICRO 0
>
> While in libav it appeared at:
> LIBAVCODEC_VERSION_MAJOR 54
> LIBAVCODEC_VERSION_MINOR 2
> LIBAVCODEC_VERSION_MICRO 0

It doesn't matter when the en- or decoder was added, it matters when the
CODEC_ID appeared in the header file. The actual presence of a codec is
deteced at runtime anyway (codecs can be disabled individually by the
ffmpeg configure script).

> I guess I could use the second one as a threshold.

If there are 2 version numbers, the higher one should of course
be preferred.

> BTW, the current versions of ffmpeg and libav are 54:45:100 and 54:24:0 respectively.
>
> One more change I will need to do to my patch is resolving the ambiguity of which implementation
 > of ProRes codec gets choosen. FFMpeg ended up with 2 versions from different authors, but support for
 > interlaced encoding is coming to only one of them (the newer one, which has other advantages).
 > Unfortunately, the codec you get from avcodec_find_encoder(CODEC_ID_PRORES) is the older one.
 > The way to get the newer one is avcodec_find_encoder_by_name("prores_kostya").

Although ffmpeg improved a LOT over the years, it's still good for some laughs from
time to time :)

> So, would you prefer a clean solution which would allow you to specify name(s) of encoders and decoders
 > in CODECIDMAP, or should I go with a quick and dirty ad-hoc solution that only deals with 2 versions of ProRes encoder?

In the hope, that it remains the only codec handled this way, a quick and dirty hack would be ok.

Burkhard

Re: [Libquicktime-devel] [PATCH] ProRes support

[Maksalov B. <bor...@ya...>]

> Up to now I completely ignored libav. Are there any differences in version numbers?
> If yes, that would be very bad.
Well, the ProRes encoder first appeared in ffmpeg at:
LIBAVCODEC_VERSION_MAJOR 53
LIBAVCODEC_VERSION_MINOR 24
LIBAVCODEC_VERSION_MICRO 0

While in libav it appeared at:
LIBAVCODEC_VERSION_MAJOR 54
LIBAVCODEC_VERSION_MINOR 2
LIBAVCODEC_VERSION_MICRO 0

I guess I could use the second one as a threshold.
BTW, the current versions of ffmpeg and libav are 54:45:100 and 54:24:0 respectively.

One more change I will need to do to my patch is resolving the ambiguity of which implementation of ProRes codec gets choosen. FFMpeg ended up with 2 versions from different authors, but support for interlaced encoding is coming to only one of them (the newer one, which has other advantages). Unfortunately, the codec you get from avcodec_find_encoder(CODEC_ID_PRORES) is the older one. The way to get the newer one is avcodec_find_encoder_by_name("prores_kostya").
So, would you prefer a clean solution which would allow you to specify name(s) of encoders and decoders in CODECIDMAP, or should I go with a quick and dirty ad-hoc solution that only deals with 2 versions of ProRes encoder?

Re: [Libquicktime-devel] [PATCH] ProRes support

[Burkhard P. <pl...@ip...>]

Hi,

Am 06.08.2012 18:14, schrieb Maksalov Boris:
>
> 06.08.2012, 18:04, "Burkhard Plaum" <pl...@ip...>:
>> Hi,
>>
>> Am 03.08.2012 13:10, schrieb Maksalov Boris:
>>
>>>   Hello,
>>>
>>>   I've implemented Apple ProRes support. The patch is attached.
>>>   Only ProRes 422 is currently supported. Libquicktime is missing an appropriate colormodel to support ProRes 4444.
>>>   You can specify ProRes 422 profile like this:
>>>   lqt_set_video_parameter(file, track, "prores_profile", (char*)"HQ");
>>>
>>>   Profile names are HQ, Standard, LT, Proxy. The default one is Standard.
>>>
>>>   The current ffmpeg / libav don't support interlaced ProRes encoding, though I've just finished working on that feature and will be sending patches to ffmpeg / libav soon. This patch for libquicktime already sets CODEC_FLAG_INTERLACED_DCT for ffmpeg, so libquicktime will be able do interlaced encoding as soon as my patches are accepted into ffmpeg.
>>
>> The patch looks ok, but the AVCODEC_SUPPORTS_PRORES macro should be set inside the .c file by
>> checking LIBAVCODEC_VERSION_INT. If every ffmpeg feature would be checked by the configure script,
>> our configure.ac would become much more messy than it already is.
>>
>> Can you make an updated patch?
>
> I can do that, but how do you handle version differences between ffmpeg and libav?

Up to now I completely ignored libav. Are there any differences in version numbers?
If yes, that would be very bad.

For me, ffmpeg.org is still the reference.

Burkhard

Re: [Libquicktime-devel] ffmpeg ffvhuff issue

[Burkhard P. <pl...@ip...>]

Am 03.08.2012 06:55, schrieb Byron Guernsey:
> I'm new to libquicktime, but have a number of the codecs working with my application to encode video.  I tried to add ffvhuff to the list, but the file being output does not look like the original images at all.  It contains a bunch of colored horizontal lines at the bottom of the screen when played back with any of the players I've tried.
>
> My understanding is that ffvhuff supports RGB888, no?

Libavcodec supports it, but our libquicktime plugin probably doesn't.

> The plugin has no encoding colormodels listed, but online discussions on ffmpeg say that 24 bit RGB colormodel is supported.

Yes, the ffmpeg source says for FFVHUFF:

PIX_FMT_YUV420P, PIX_FMT_YUV422P, PIX_FMT_RGB24, PIX_FMT_RGB32

> The resultant file, when inspected with ffmpeg reports:
>
>      Stream #0:0(eng): Video: ffvhuff (FFVH / 0x48564646), yuv420p, 1600x1200, 343437 kb/s, 30 fps, 30 tbr, 30 tbn, 30 tbc
>
> Note the yuv420p color model.  The encoding is done using BC_RGB888 as the color model, and I expected to result to be rgb24.

If libquicktime assumes that only BC_YUV420P is supported, it will do inline conversion.

> Am I mistaken on ffvhuff supporting RGB888 or is this a bug in the plugin?

Not a bug but a missing feature in the plugin. I would try adding BC_YUV420P, BC_YUV422P and BC_RGB888
to the encoding colormodels. If that works for you, you can send a patch.

Burkhard

Re: [Libquicktime-devel] [PATCH] ProRes support

[Maksalov B. <bor...@ya...>]

06.08.2012, 18:04, "Burkhard Plaum" <pl...@ip...>:
> Hi,
>
> Am 03.08.2012 13:10, schrieb Maksalov Boris:
>
>>  Hello,
>>
>>  I've implemented Apple ProRes support. The patch is attached.
>>  Only ProRes 422 is currently supported. Libquicktime is missing an appropriate colormodel to support ProRes 4444.
>>  You can specify ProRes 422 profile like this:
>>  lqt_set_video_parameter(file, track, "prores_profile", (char*)"HQ");
>>
>>  Profile names are HQ, Standard, LT, Proxy. The default one is Standard.
>>
>>  The current ffmpeg / libav don't support interlaced ProRes encoding, though I've just finished working on that feature and will be sending patches to ffmpeg / libav soon. This patch for libquicktime already sets CODEC_FLAG_INTERLACED_DCT for ffmpeg, so libquicktime will be able do interlaced encoding as soon as my patches are accepted into ffmpeg.
>
> The patch looks ok, but the AVCODEC_SUPPORTS_PRORES macro should be set inside the .c file by
> checking LIBAVCODEC_VERSION_INT. If every ffmpeg feature would be checked by the configure script,
> our configure.ac would become much more messy than it already is.
>
> Can you make an updated patch?

I can do that, but how do you handle version differences between ffmpeg and libav?

Re: [Libquicktime-devel] [PATCH] ProRes support

[Burkhard P. <pl...@ip...>]

Hi,

Am 03.08.2012 13:10, schrieb Maksalov Boris:
> Hello,
>
> I've implemented Apple ProRes support. The patch is attached.
> Only ProRes 422 is currently supported. Libquicktime is missing an appropriate colormodel to support ProRes 4444.
> You can specify ProRes 422 profile like this:
> lqt_set_video_parameter(file, track, "prores_profile", (char*)"HQ");
>
> Profile names are HQ, Standard, LT, Proxy. The default one is Standard.
>
> The current ffmpeg / libav don't support interlaced ProRes encoding, though I've just finished working on that feature and will be sending patches to ffmpeg / libav soon. This patch for libquicktime already sets CODEC_FLAG_INTERLACED_DCT for ffmpeg, so libquicktime will be able do interlaced encoding as soon as my patches are accepted into ffmpeg.

The patch looks ok, but the AVCODEC_SUPPORTS_PRORES macro should be set inside the .c file by
checking LIBAVCODEC_VERSION_INT. If every ffmpeg feature would be checked by the configure script,
our configure.ac would become much more messy than it already is.

Can you make an updated patch?

Burkhard

Re: [Libquicktime-devel] [PATCH] ProRes support

[Peter D. <li...@pd...>]

Maksalov Boris wrote:
> Hello,
>
> I've implemented Apple ProRes support. The patch is attached.
> Only ProRes 422 is currently supported. Libquicktime is missing an 
> appropriate colormodel to support ProRes 4444.
> You can specify ProRes 422 profile like this:
> lqt_set_video_parameter(file, track, "prores_profile", (char*)"HQ");
>
> Profile names are HQ, Standard, LT, Proxy. The default one is Standard.

Thank you very much, this was exactly what I needed. Saving ProRes does emit 
a warning that:

[codecs] Warning: Video codec and container are not known to be compatible. 
File might be playable by libquicktime only.

but that's a minor issue. Thank you again for your work. 

[Libquicktime-devel] [PATCH] ProRes support

[Maksalov B. <bor...@ya...>]

Hello,

I've implemented Apple ProRes support. The patch is attached.
Only ProRes 422 is currently supported. Libquicktime is missing an appropriate colormodel to support ProRes 4444.
You can specify ProRes 422 profile like this:
lqt_set_video_parameter(file, track, "prores_profile", (char*)"HQ");

Profile names are HQ, Standard, LT, Proxy. The default one is Standard.

The current ffmpeg / libav don't support interlaced ProRes encoding, though I've just finished working on that feature and will be sending patches to ffmpeg / libav soon. This patch for libquicktime already sets CODEC_FLAG_INTERLACED_DCT for ffmpeg, so libquicktime will be able do interlaced encoding as soon as my patches are accepted into ffmpeg.


Regards,
    Boris Maksalov

[Libquicktime-devel] ffmpeg ffvhuff issue

[Byron G. <byr...@ik...>]

I'm new to libquicktime, but have a number of the codecs working with my application to encode video.  I tried to add ffvhuff to the list, but the file being output does not look like the original images at all.  It contains a bunch of colored horizontal lines at the bottom of the screen when played back with any of the players I've tried.

My understanding is that ffvhuff supports RGB888, no?  The plugin has no encoding colormodels listed, but online discussions on ffmpeg say that 24 bit RGB colormodel is supported.

The resultant file, when inspected with ffmpeg reports:

    Stream #0:0(eng): Video: ffvhuff (FFVH / 0x48564646), yuv420p, 1600x1200, 343437 kb/s, 30 fps, 30 tbr, 30 tbn, 30 tbc

Note the yuv420p color model.  The encoding is done using BC_RGB888 as the color model, and I expected to result to be rgb24.

Am I mistaken on ffvhuff supporting RGB888 or is this a bug in the plugin?

thanks,
Byron

[Libquicktime-devel] Prores support?

[Peter D. <li...@pd...>]

Hello,

I have an application that uses libquicktime under Linux, and my users are 
asking me to support Prores .mov files. Recent FFmpeg versions do support 
Prores, so I naively assumed that a libquicktime that is linked against 
FFmpeg 0.11.1 (for instance) will be able to decode Prores, but alas, it 
does not. Is there a reason for that?

Thanks in advance.

Re: [Libquicktime-devel] [PATCH] Multithreaded decoding

[Burkhard P. <pl...@ip...>]

Hi,

Am 28.06.2012 16:20, schrieb Joseph Artsimovich:
> On 28/06/2012 14:28, Burkhard Plaum wrote:
>>
>> Then you found a bug.
>>
>>> The
>>> problem is lqt_set_default_video_parameters() is called too late in
>>> do_open(), that is after quicktime_read_info(), which ends up calling
>>> avcodec_open(). Calling it before quicktime_read_info() is probably unsafe.
>>
>> I don't think so. The problem was indeed, that the colormodel was detected way too
>> early. Could you please try the attached patch against cvs?
> It works with your patch. Thanks.

Applied

>>> There were also PARAM_THREAD_COUNT missing here and there, but that was
>>> easy to fix.
>>
>> A patch for this is welcome. When I touched these parameters the last time,
>> multithreaded decoding wasn't supported by ffmpeg.
> Attached.

Applied

Burkhard

Re: [Libquicktime-devel] [PATCH] Multithreaded decoding

[Joseph A. <jo...@mi...>]

On 28/06/2012 14:28, Burkhard Plaum wrote:
>
> Then you found a bug.
>
>> The
>> problem is lqt_set_default_video_parameters() is called too late in
>> do_open(), that is after quicktime_read_info(), which ends up calling
>> avcodec_open(). Calling it before quicktime_read_info() is probably 
>> unsafe.
>
> I don't think so. The problem was indeed, that the colormodel was 
> detected way too
> early. Could you please try the attached patch against cvs?
It works with your patch. Thanks.

>
>> There were also PARAM_THREAD_COUNT missing here and there, but that was
>> easy to fix.
>
> A patch for this is welcome. When I touched these parameters the last 
> time,
> multithreaded decoding wasn't supported by ffmpeg.
Attached.

-- 
Joseph Artsimovich
Senior C++ Applications Developer
MirriAd Ltd

Re: [Libquicktime-devel] [PATCH] Multithreaded decoding

[Burkhard P. <pl...@ip...>]

Hi,

Am 28.06.2012 13:11, schrieb Joseph Artsimovich:
> On 27/06/2012 16:22, Burkhard Plaum wrote:
>> Hi,
>>
>> Am 25.06.2012 17:47, schrieb Joseph Artsimovich:
>>
>>>> What prevents you from calling lqt_set_default_parameter() before opening the file?
>>> I didn't know about that one. If I knew, I probably would just use that.
>> Well now you know :)
> I tried to implement it that way, but unfortunately it doesn't work.

Then you found a bug.

> The
> problem is lqt_set_default_video_parameters() is called too late in
> do_open(), that is after quicktime_read_info(), which ends up calling
> avcodec_open(). Calling it before quicktime_read_info() is probably unsafe.

I don't think so. The problem was indeed, that the colormodel was detected way too
early. Could you please try the attached patch against cvs?

> There were also PARAM_THREAD_COUNT missing here and there, but that was
> easy to fix.

A patch for this is welcome. When I touched these parameters the last time,
multithreaded decoding wasn't supported by ffmpeg.

Of course one should look at the ffmpeg sources and make sure the codecs are
really multithreaded before blindly adding bogus parameters.

> In addition, the client code ends up being horrendous:
>
> lqt_parameter_value_t thread_count = {};
> thread_count.val_int = 4;
> lqt_codec_info_t** codecs = lqt_query_registry(1, 1, 1, 1);
> for (int i = 0; codecs[i]; ++i) {
>       lqt_codec_info_t* codec = codecs[i];
>       for (int j = 0; j < codec->num_decoding_parameters; ++j) {
>           if (!strcmp(codec->decoding_parameters[j].name,
> "ff_thread_count")) {
>               lqt_set_default_parameter(codec->type, /*encode=*/0,
> codec->name, "ff_thread_count", &thread_count);
>           }
>       }
>       for (int j = 0; j < codec->num_encoding_parameters; ++j) {
>           if (!strcmp(codec->encoding_parameters[j].name,
> "ff_thread_count")) {
>               lqt_set_default_parameter(codec->type, /*encode=*/1,
> codec->name, "ff_thread_count", &thread_count);
>           }
>       }
> }
> lqt_destroy_codec_info(codecs);

Well the most horrendous thing I see is that thread_count is hardcoded to 4
and that you also set the encoding parameter, which isn't necessary and
makes the loop almost twice as long as it needs to be.

Also if you want to contribute an utility function, which sets a particular
default parameter for all codecs, which support it, it wouldn't hurt and your
code becomes a one-liner.

Burkhard

Re: [Libquicktime-devel] [PATCH] Multithreaded decoding

[Joseph A. <jo...@mi...>]

On 27/06/2012 16:22, Burkhard Plaum wrote:
> Hi,
>
> Am 25.06.2012 17:47, schrieb Joseph Artsimovich:
>
>>> What prevents you from calling lqt_set_default_parameter() before opening the file?
>> I didn't know about that one. If I knew, I probably would just use that.
> Well now you know :)
I tried to implement it that way, but unfortunately it doesn't work. The 
problem is lqt_set_default_video_parameters() is called too late in 
do_open(), that is after quicktime_read_info(), which ends up calling 
avcodec_open(). Calling it before quicktime_read_info() is probably unsafe.
There were also PARAM_THREAD_COUNT missing here and there, but that was 
easy to fix.

In addition, the client code ends up being horrendous:

lqt_parameter_value_t thread_count = {};
thread_count.val_int = 4;
lqt_codec_info_t** codecs = lqt_query_registry(1, 1, 1, 1);
for (int i = 0; codecs[i]; ++i) {
     lqt_codec_info_t* codec = codecs[i];
     for (int j = 0; j < codec->num_decoding_parameters; ++j) {
         if (!strcmp(codec->decoding_parameters[j].name, 
"ff_thread_count")) {
             lqt_set_default_parameter(codec->type, /*encode=*/0, 
codec->name, "ff_thread_count", &thread_count);
         }
     }
     for (int j = 0; j < codec->num_encoding_parameters; ++j) {
         if (!strcmp(codec->encoding_parameters[j].name, 
"ff_thread_count")) {
             lqt_set_default_parameter(codec->type, /*encode=*/1, 
codec->name, "ff_thread_count", &thread_count);
         }
     }
}
lqt_destroy_codec_info(codecs);


-- 
Joseph Artsimovich
Senior C++ Applications Developer
MirriAd Ltd

Re: [Libquicktime-devel] [PATCH] Multithreaded decoding

[Burkhard P. <pl...@ip...>]

Hi,

Am 25.06.2012 17:47, schrieb Joseph Artsimovich:

>> What prevents you from calling lqt_set_default_parameter() before opening the file?
> I didn't know about that one. If I knew, I probably would just use that.

Well now you know :)

> However, as lqt_set_default_parameter() requires the codec's short name,
> you would have to open the file twice: once to get the codec info and
> another time to do the actual decoding. On second thought, I could
> enumerate all codecs and set this parameter for all of them. Doable for
> sure, but a bit cruel on the users.

Or you ship a custom codec configuration, which sets this parameter.

> My patch on the contrary, while introducing some complexity internally,

Way too much complexity for my taste. And it it works only for thread_count.
If it would at least be generic enough to support other (future-) decoding
parameters, but in the current version it's unacceptable.

> doesn't tax the user at all. You just do:
>
> quicktime_t* file = lqt_open_read(fname);
> int thread_count = 4;
> quicktime_set_parameter(file, "ff_thread_count", &thread_count);

Well that's one user of the library (i.e. you), and IMO not a typical one.

In a typical application, thread_count (and prossibly some other parameters)
are configurable by the application user in some way (e.g. commandline options).
So a more generic parameter passing mechanism would be necessary anyway. And
while this is implemented, it doesn't matter if parameters are set globally
(via lqt_set_default_parameter()) or for each opened file.

> That is, just like you do for encoding.
>
>> It can even be done globally in the application (assuming the desired thread count is always the same).
>>
>> The only clean solution would be something like:
>>
>> /* Create handle */
>> quicktime_t * lqt = lqt_create();
>>
>> /* Set parameters */
>> // ....
>>
>> /* Open for reading */
>> lqt_open_read(lqt, file);
>>
>> That's how I did things in gmerlin. In libquicktime it's more difficult to implement
>> without breaking backwards compatibility. But if someone wants to implement such a solution
>> it would be better than working around architecture flaws.
> Yes, that would be the proper way. Unfortunately, it's too much effort
> to implement

If I'd had this argument while working on lqt, the code would still be as messy as it
used to be.

> and it will break the API.

Burkhard

Re: [Libquicktime-devel] [PATCH] Multithreaded decoding

[Joseph A. <jo...@mi...>]

On 25/06/2012 14:51, Burkhard Plaum wrote:
> Hi,
>
> Am 22.06.2012 16:05, schrieb Joseph Artsimovich:
>> Hi,
>>
>> The problem with getting multithreaded decoding working is a chicken and egg problem:
>> 1. libavcodec wants thread_count to be set before calling avcodec_open().
>> 2. avcodec_open() is called from within lqt_open_read() to be able to extract the colormodel.
>> 3. To set a parameter in libquicktime, you need a handle returned by lqt_open_read().
>> So, when a user has the opportunity to set the thread count, it's already too late, as avcodec_open() was already called.
>>
>> The only solution seems to be reopening the decoder when the user sets the thread count. The attached patch implements
>   > this solution.
>
> What prevents you from calling lqt_set_default_parameter() before opening the file?
I didn't know about that one. If I knew, I probably would just use that. 
However, as lqt_set_default_parameter() requires the codec's short name, 
you would have to open the file twice: once to get the codec info and 
another time to do the actual decoding. On second thought, I could 
enumerate all codecs and set this parameter for all of them. Doable for 
sure, but a bit cruel on the users.
My patch on the contrary, while introducing some complexity internally, 
doesn't tax the user at all. You just do:

quicktime_t* file = lqt_open_read(fname);
int thread_count = 4;
quicktime_set_parameter(file, "ff_thread_count", &thread_count);

That is, just like you do for encoding.

> It can even be done globally in the application (assuming the desired thread count is always the same).
>
> The only clean solution would be something like:
>
> /* Create handle */
> quicktime_t * lqt = lqt_create();
>
> /* Set parameters */
> // ....
>
> /* Open for reading */
> lqt_open_read(lqt, file);
>
> That's how I did things in gmerlin. In libquicktime it's more difficult to implement
> without breaking backwards compatibility. But if someone wants to implement such a solution
> it would be better than working around architecture flaws.
Yes, that would be the proper way. Unfortunately, it's too much effort 
to implement and it will break the API.

-- 
Joseph Artsimovich
Senior C++ Applications Developer
MirriAd Ltd

Re: [Libquicktime-devel] [PATCH] Multithreaded decoding

[Burkhard P. <pl...@ip...>]

Hi,

Am 22.06.2012 16:05, schrieb Joseph Artsimovich:
> Hi,
>
> The problem with getting multithreaded decoding working is a chicken and egg problem:
> 1. libavcodec wants thread_count to be set before calling avcodec_open().
> 2. avcodec_open() is called from within lqt_open_read() to be able to extract the colormodel.
> 3. To set a parameter in libquicktime, you need a handle returned by lqt_open_read().
> So, when a user has the opportunity to set the thread count, it's already too late, as avcodec_open() was already called.
>
> The only solution seems to be reopening the decoder when the user sets the thread count. The attached patch implements
 > this solution.

What prevents you from calling lqt_set_default_parameter() before opening the file?
It can even be done globally in the application (assuming the desired thread count is always the same).

The only clean solution would be something like:

/* Create handle */
quicktime_t * lqt = lqt_create();

/* Set parameters */
// ....

/* Open for reading */
lqt_open_read(lqt, file);

That's how I did things in gmerlin. In libquicktime it's more difficult to implement
without breaking backwards compatibility. But if someone wants to implement such a solution
it would be better than working around architecture flaws.

Burkhard

Re: [Libquicktime-devel] [PATCH] Multithreaded decoding

[Joseph A. <jo...@mi...>]

Forgot to reset decoding_delay. Updated version attached.

-- 
Joseph Artsimovich
Senior C++ Applications Developer
MirriAd Ltd

[Libquicktime-devel] [PATCH] Multithreaded decoding

[Joseph A. <jo...@mi...>]

Hi,

The problem with getting multithreaded decoding working is a chicken and 
egg problem:
1. libavcodec wants thread_count to be set before calling avcodec_open().
2. avcodec_open() is called from within lqt_open_read() to be able to 
extract the colormodel.
3. To set a parameter in libquicktime, you need a handle returned by 
lqt_open_read().
So, when a user has the opportunity to set the thread count, it's 
already too late, as avcodec_open() was already called.

The only solution seems to be reopening the decoder when the user sets 
the thread count. The attached patch implements this solution.

-- 
Joseph Artsimovich
Senior C++ Applications Developer
MirriAd Ltd

Re: [Libquicktime-devel] [PATCH] Unified seeking using stps and ctts

[Joseph A. <jo...@mi...>]

On 12/06/2012 18:19, Burkhard Plaum wrote:
> Hi,
>
> Am 12.06.2012 10:31, schrieb Joseph Artsimovich:
>> On 11/06/2012 18:39, Burkhard Plaum wrote:
> [...]
>>> I would just update the old keyframe detection functions such that they look in stss and stps.
>> That's not enough. The old code never goes to the previous GOP, which
>> you have to do with open GOPs if you want frame accurate seeking.
> If you search for a frame X in *display* order, you seek to the
> highest numbered keyframe (in *display* order) which is<= X and
> start to decode from there. This is true for open and closed GOPS.
If libquicktime had functionality to seek to a frame in display order, 
that might indeed be a good approach.

>> Besides, the new code is better commented.
> Well I applied this, but that doesn't mean I'm completely happy with the
> overall situation (not your fault, it's libquicktimes architecture).
Thanks for accepting my patch.

>
> The problem is that each libquicktime codec does things, for which other
> libraries have a separate demuxer layer. I can remember to straighten the
> initial mess a bit by changing each codec to use lqt_read_video_frame() for
> getting a compressed video chunk (before the mess was much worse).
> This could theoretically also return the frame type (I-frame of open/closed
> GOP, B-frame, P-frame etc) in a generic manner.
> quicktime_get_keyframe_before() could be fixed to handle frames in display
> order and so on.
Are plugins other than ffmpeg still relevent? If we had only one plugin, 
there would be no code duplication.

>
> Since I don't use libquicktime privately that much anymore I'm not motivated
> to do larger changes to the architecture. But if someone volunteers to clean
> things up a bit I'd be there to answer questions regarding architecture- or
> fileformat issues.
My management probably wouldn't want to allocate any of my time on that. 
So I am in "do no harm" only mode.

-- 
Joseph Artsimovich
Senior C++ Applications Developer
MirriAd Ltd

Re: [Libquicktime-devel] [PATCH] Unified seeking using stps and ctts

[Baptiste C. <bap...@gm...>]

Hi Burkhard,

On 06/11/2012 10:39 AM, Burkhard Plaum wrote:
> [...]
> 
> Also did you consider that the ctts values are different for mov and mp4?
> In mp4 files the ctts values are offset such, that they can never get negative.
> 
> The quicktime approach (negative offsets allowed) has the advantage, that
> we simply do:
> 
> DTS (from stts) + CTS (from ctts) = PTS (starting with zero)
> 
> But the MPEG-4 ISO standard says that the CTS can never be negative (because a frame can't
> be displayed before it's decoded).
> I leave the arguing about the correct approach to the philosophers, but lqt needs to support both.

They can also be negative in mp4 if the atom is using version 1 according to one revision of the
specs, it would match mov in that case. Files can also have an atom cslg or something, supplying an
offset to apply to timestamps that will make both pts and dts positive.

> [...]
> 
> Which (for me) proves once again, that we don't need to make a difference between stps and stss.
> H.264 can have open GOPS too (and even worse things) and doesn't need that discinction.

This is tricky. Using Open GOP with Quicktime have interesting effects,
using stss for them confuses Quicktime, but somehow last time I tested, Quicktime can
handle them somewhat with stps, so I think Quicktime is aware about it.

-- 
Baptiste COUDURIER
Key fingerprint          8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
FFmpeg maintainer                           http://www.ffmpeg.org

Re: [Libquicktime-devel] [PATCH] Unified seeking using stps and ctts

[Burkhard P. <pl...@ip...>]

Hi,

Am 12.06.2012 10:31, schrieb Joseph Artsimovich:
> On 11/06/2012 18:39, Burkhard Plaum wrote:
[...]
>>
>> I would just update the old keyframe detection functions such that they look in stss and stps.
> That's not enough. The old code never goes to the previous GOP, which
> you have to do with open GOPs if you want frame accurate seeking.

If you search for a frame X in *display* order, you seek to the
highest numbered keyframe (in *display* order) which is <= X and
start to decode from there. This is true for open and closed GOPS.

If you seek for a frame X in *coded* order, then you might go back
to the previous GOP, that's right.

Open GOP (coding order):    IPBBPBBPBBIBB
Open GOP (display order):   IBBPBBPBBPBBI

Closed GOP (coding order):  IPBBPBBPBBPBI
Close GOP  (display order): IBBPBBPBBPBPI

That's why I prefer seeking by PTS value, since it's always unambiguous
and makes more sense if the framerate is variable.

These things could be clarified (display/coding order), fixed and properly
documented, ideally in the lqt core (see below).

> Besides, the new code is better commented.

Well I applied this, but that doesn't mean I'm completely happy with the
overall situation (not your fault, it's libquicktimes architecture).

The problem is that each libquicktime codec does things, for which other
libraries have a separate demuxer layer. I can remember to straighten the
initial mess a bit by changing each codec to use lqt_read_video_frame() for
getting a compressed video chunk (before the mess was much worse).
This could theoretically also return the frame type (I-frame of open/closed
GOP, B-frame, P-frame etc) in a generic manner.
quicktime_get_keyframe_before() could be fixed to handle frames in display
order and so on.

Since I don't use libquicktime privately that much anymore I'm not motivated
to do larger changes to the architecture. But if someone volunteers to clean
things up a bit I'd be there to answer questions regarding architecture- or
fileformat issues.

Burkhard

Re: [Libquicktime-devel] Washed out colors when reading DnxHD video

[Phoenix at w. <pho...@gm...>]

On Fri, Jun 1, 2012 at 9:45 AM, Joseph Artsimovich wrote:

> Looks like FFMpeg generates a video range YUV file but puts in ACLR
> indicating it's a full range one.
> Now, when FFMpeg decodes DNxHD, it ignores ACLR atom, which is why it
> renders it "correctly".
> The old libquicktime didn't interpret ACLR either.
> Basically, the "correct" rendering is actually incorrect one, and
> libquicktime rendering should (mostly) agree with Apple's QuickTime player.
>

Ok, thanks for your answer and sorry for the delay of mine.

We did more tests with Apple Quicktime Pro on Mac.
We are confused. There are a lot of options to generate a DNxHD movie, but
we let everything by default.
We only focused on the main option that seemed important for our issue:
Rec709 or sRGB.
We tried both, and each time, video is washed out (compared to our original
sRGB PNG source images) when reading it back with the same Quicktime Pro
Player.
On both videos, Qtdump indicates Full Range in ACLR.

If we read those videos with others players on Mac, sometimes, videos seems
correct (colors are transformed back and match aproximatively source
images), sometimes not at all.
Apple Quicktime Pro is supposed to be the reference, but we didn't
understood the logic. Each software seems to have a custom behavior.

On Linux, libquicktime decodes those videos washed out, but ffplay always
displays video correctly and colors are exactly the same compared to our
PNG images.

We also checked our pipeline used to generate movies on Linux. We found
that the ffmpeg version we used to encode DNxHD video was pretty old. We
tried encoding using ffmpeg 0.10.2 (the version actually linked to our
libquicktime compiled library).
In this case, the DNxHD generated is decoded correctly via libquicktime
1.2.4, displayed correctly with ffplay (but still washed out with Quicktime
Pro).
And there is no difference in the qtdump output with the output from the
original washed out video.

So what can we conclude?
Just use latest libquicktime and ffmpeg, stay on linux, don't think about
Quicktime and pray? ;-)
Or do you think we missed something?

Thanks a lot.
-- 
Phoenix

Re: [Libquicktime-devel] [PATCH] Unified seeking using stps and ctts

[Joseph A. <jo...@mi...>]

On 11/06/2012 18:39, Burkhard Plaum wrote:
> Hi,
>
> Am 11.06.2012 16:37, schrieb Joseph Artsimovich:
>> Hi,
>>
>> Having learned about the existence of ctts table, I modified my seeking code to
>   >  take advantage of it and remove bitstream parsing.
>
> Very good :)
>
>   >  Then I modified it to work for any
>   >  case: whether ctts / stps are present or not. Then I replaced the old resync_ffmpeg() with my version.
>
> Do I understand it right, that your B-frame detector just looks up the
> proper table entry (for the case of multiple samples with the same offset)?
> It's because in the struct quicktime_video_map_t (qtprivate.h) we already have:
>
>     int64_t stts_index;
>     int64_t stts_count;
>
>     int64_t ctts_index;
>     int64_t ctts_count;
I don't think these are kept up-to date when decoding. Specifically, I 
don't see any code that would update them after seeking. Well, I guess I 
just wrote the code that can be used to update them, but I still like 
the ability to have more than one bframe detector pointed at different 
frames, and be sure those have no side effects and aren't modified 
behind my back.

> Also did you consider that the ctts values are different for mov and mp4?
> In mp4 files the ctts values are offset such, that they can never get negative.
>
> The quicktime approach (negative offsets allowed) has the advantage, that
> we simply do:
>
> DTS (from stts) + CTS (from ctts) = PTS (starting with zero)
>
> But the MPEG-4 ISO standard says that the CTS can never be negative (because a frame can't
> be displayed before it's decoded).
> I leave the arguing about the correct approach to the philosophers, but lqt needs to support both.
>
> Which would mean that we have a B-frame if:
>
> ctts->table[index].sample_duration - ctts->table[0]<  0
I didn't know that about MP4. In this case, the following line in my patch:
return ctx->ctts->table[ctx->ctts_entry_idx].sample_duration < 0;
should be replaced with:
return ctx->ctts->table[ctx->ctts_entry_idx].sample_duration < 
ctx->ctts->table[0].sample_duration;

>> I tested it with a few codecs, and everything worked, including an H.264 sample with ctts present but
>   >  stps missing. I couldn't find any samples besides XDCAM ones with both being present.
>
> Which (for me) proves once again, that we don't need to make a difference between stps and stss.
> H.264 can have open GOPS too (and even worse things) and doesn't need that discinction.
In my opinion it merely means that people are trying to stay away from 
open GOPs, as they cause seeking problems with some software.
And yes, you can treat stts and stps the same during decoding, but then 
you loose some advantages, like knowing immediately (when seeking to an 
entry in stts) that you don't need to go to the previous GOP.

>
> I would just update the old keyframe detection functions such that they look in stss and stps.
That's not enough. The old code never goes to the previous GOP, which 
you have to do with open GOPs if you want frame accurate seeking. 
Besides, the new code is better commented.


-- 
Joseph Artsimovich
Senior C++ Applications Developer
MirriAd Ltd