Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#11018 closed bug (fixed)

[Media Kit]: BMediaFormats::GetFormatFor doesn't find any formats anymore

Reported by: colin Owned by: axeld
Priority: normal Milestone: Unscheduled
Component: Kits/Media Kit Version: R1/Development
Keywords: BMediaFormats, GetFormatFor, DVB, ffmpeg Cc:
Blocked By: Blocking:
Has a Patch: no Platform: All

Description

While working on bringing DVB support back to life I encountered a bug that was introduced by the commits [a] and [b].

The bug hides in the realm of an app requesting a specific decoder (MPEG2 video and MP3 audio in the case of DVB support). When the app requests these decoders via BMediaFormats::GetFormatFor() (see [c] for a code snippet in the dvb.media_addon) it won't find any decoders, because the AddOnManager ([d]) didn't load the ffmpeg media plugin and its codecs yet.

I've commited two unit tests (MPEG2 video decoding [f] and MP3 video decoding [g]) to the haiku repo, that demonstrate the bug. For example: Just set a breakpoint at [h] and see that the status returned is not B_OK as one would expect. When you step in to the call to formats.GetFormatFor() one line earlier, you will see that there are no codecs loaded yet.

I tried the following solutions:

  1. Load the plugins (_RegisterAddons()) within the constructor of the AddOnManager.
  2. Call AddOnManager::LoadState() (After reintroducing the Method LoadState() in the AddOnManager) from within the constructor of BMediaFormats.

Both solutions successfully (as in tried and confirmed) emulate the behavior of AddOnManager when it was still part of the media_server. Back then the media_server issued a call to AddOnManager::LoadState() in the method ReadyToRun() (see [e]) resulting in the loading of all ffmpeg's codecs. But both solutions smell bad to me. Unfortunately I can't really tell why it smells bad to me. As of now I would rather prefer the former way of handling media plugins via the media_server.

There exists a mailing list discussion thread [i] with Adrien explaining the reason behind both commits ([a] and [b]) and Axel outlining a possible solution on the server side.

References

[a] Move media plug-in support to application side. http://cgit.haiku-os.org/haiku/commit/src/kits/media?id=2feaa37f244d707251f7fe1184ce4f7d30251e2d [b] Urpdate AddOnManager and FormatManager for Media Kit. http://cgit.haiku-os.org/haiku/commit/src/kits/media?id=bf3b475c3838eb2da1f4a97b214535698902380b [c] dvb.media_addon: https://github.com/haiku/haiku/blob/master/src/add-ons/media/media-add-ons/dvb/MediaFormat.cpp#L160 [d] Media Kit's AddOnManager: https://github.com/haiku/haiku/blob/master/src/kits/media/AddOnManager.cpp [e] media_server's old code location of loading all ffmpeg codecs: http://cgit.haiku-os.org/haiku/tree/src/servers/media/media_server.cpp?id=hrev46599#n152 [f] mpeg2_decoder_test: http://cgit.haiku-os.org/haiku/tree/src/tests/kits/media/mpeg2_decoder_test?id=hrev47470 [g] mp3_decoder_test: http://cgit.haiku-os.org/haiku/tree/src/tests/kits/media/mp3_decoder_test?id=hrev47470 [h] breakpoint in unit test mpeg2_decoder_test where the bug shows itself: http://cgit.haiku-os.org/haiku/tree/src/tests/kits/media/mpeg2_decoder_test/mpeg2_decoder_test.cpp?id=hrev47470#n189 [i] mailing list discussion about this bug: http://www.freelists.org/post/haiku-development/Media-plugin-support-to-message-or-not-to-message

Change History (5)

comment:1 Changed 5 years ago by colin

Workaround

You can work around this bug by calling get_next_encoder (defined in BMediaFormats.h) before calling a method of BMediaFormats.

The workaround triggers the loading of all media plugins prior to using methods of class BMediaFormats. Using the function get_next_encoder() is used because of the following facts

  1. It is publicly available and thus can be used by 3rd party apps, too.
  2. It is already available by including BMediaFormats.h, so there is no need to include another header for this workaround.

You can see the workaround applied here: http://cgit.haiku-os.org/haiku/commit/?id=hrev47475

comment:2 Changed 5 years ago by pulkomandy

Resolution: fixed
Status: newclosed

Fixed in hrev48070. You are right that registering the add-ons in the constructors is not a good idea. I designed this so the loading can be delayed as much as possible. This way we keep the application start time short, and the scanning only happens when needed because the app calls one of the media kit functions.

I made RegisterAddOns public in AddOnManager and called it from MediaFormats format update function. Since RegisterAddOns does nothing except the first time it's called, there is no problem in doing this.

Thanks for investigating this. I can confirm the issue is solved as the mp3_decoder_test does not crash anymore (with the workaround removed). However it seems ffmpeg has trouble decoding the file.

comment:3 Changed 5 years ago by colin

@pulkomandy: Could you please remove the workaround code from the mpeg2_decoder_test and from the DVBMediaAddon, too, then.

I would do it myself, but I'm still not fully recovered from my drive crash yet and thus cannot compile, test and commit :(

comment:4 Changed 5 years ago by pulkomandy

Done in hrev48076. (untested - I don't have DVB hardware)

comment:5 Changed 5 years ago by colin

Thank you! I will cover the testing part and fix it if there was something broken at BeGeistert 28 :)

Note: See TracTickets for help on using tickets.