Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,10 @@ void GStreamerRegistryScanner::initializeDecoders(const GStreamerRegistryScanner
// USAC AOT, is not yet widely available enough to be enabled by default.
const char* value = g_getenv("WEBKIT_GST_CAN_PLAY_USAC");
bool canPlayUsac = value && (!g_strcmp0(value, "true") || !g_strcmp0(value, "1"));
if (!value) {
// Env is not set. Fall back to gst elements query
canPlayUsac = factories.hasElementForMediaType(ElementFactories::Type::AudioDecoder, "audio/mpeg, mpegversion=(int)4, stream-format=(string)usac"_s).isSupported;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What element is this?

I think a change like this can be upstreamed, but given the unusual -- and likely problematic -- value for stream-format, I think it's best if we leave a comment mentioning it by name, so that next time we know why this check is there and don't accidentally remove it.

I also noticed that in previous messages, and in the commit message you said "audio sinks" but the check is for an audio decoder. Was that a mistake or does the element in question happen to be both a decoder a sink element (which is possible, depending how the hardware platform API works)?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is Amlogic "amlhalasink" element that is both decoder and sink. The commit message is misleading indeed

amlhalasink:   Long-name                AmlHalAsink
amlhalasink:   Klass                    Decoder/Sink/Audio
...
amlhalasink: Pad Templates:
amlhalasink:   SINK template: 'sink'
amlhalasink:     Availability: Always
amlhalasink:     Capabilities:
...
amlhalasink:       audio/x-ac4
amlhalasink:       audio/mpeg
amlhalasink:             mpegversion: 4
amlhalasink:           stream-format: { (string)loas, (string)usac }

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the same with MediaTek audio sink "mtkaudiosink" which is also decoder and sink at once

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the same with MediaTek

So both amlhalasink and mtkaudiosink report support for stream-format=usac?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, they both do that actually but it's not a coincidence because mtk had been adjusted for our needs based on amlogic version kind of. So possibly that's why they use the same approach

}
if (canPlayUsac)
m_decoderCodecMap.add("mp4a.40.42"_s, result); // MPEG-4 Extended HE-AAC and xHE-AAC (USAC AOT)
}
Expand Down