Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for AVMetrics #925

Open
4 of 10 tasks
defagos opened this issue Jul 5, 2024 · 9 comments
Open
4 of 10 tasks

Add support for AVMetrics #925

defagos opened this issue Jul 5, 2024 · 9 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@defagos
Copy link
Member

defagos commented Jul 5, 2024

As a Pillarbox developer I want to benefit from new APIs introduced in iOS and tvOS 18 to better understand the behavior of Pillarbox.

Remarks

timeSpentInInitialStartup provided in AVMetricPlayerItemPlaybackSummaryEvent matches timeTaken from AVMetricPlayerItemInitialLikelyToKeepUpEvent. These values are smaller than the values we get when measuring the total startup time according to recommendations provided at WWDC. It is also the same value as provided by the first event startupTime in the access log.

Hints

  • AVMetrics are missing in iOS 18 beta 1.
  • In beta 2 they are only observed on device.
  • The same metric AVMetricContentKeyRequestEvent provides the key system used to load the content. It can be compared to clearKey, authorizationToken or fairPlayStreaming to find its type, letting us guess which timing we need to provide. Note that our token-protected streams are currently reported as using clearKey.

Acceptance criteria

  • Native AVPlayer metrics are provided in Pillarbox player metric events.

Tasks

  • Deliver native AVMetrics events.
  • Check consistency between APIs (e.g. stalls or fatal / non-fatal errors).
  • Mark custom events with a native counterpart as deprecated on iOS / tvOS 18+.
  • Check if content key retrieval metrics (for Akamai token resource loading) match timings we obtain with logs in the resource loader delegate implementation itself.
  • Provide resource loading / content session metrics in Pillarbox reports.
  • Make sure the code compiles with Xcode 15 until Xcode 16 is released (check that compiler version >= 6.0).
  • Display relevant metrics in demo metrics view (e.g. resource loading / content key session).
  • Decide if we want to have the same behavior as our metric event API (collect events for each item right from the start; this would force AVMetricEvent to be collected even if not needed, though). Or we could just collect some essential events and discard less relevant ones, at least for tracker implementation purposes.
  • Check behavior in playlists.
  • Check we can subscribe to native metric event publishers from different locations at the same time (to check if this is beta issue, consume two metric async streams from two different locations in a sample project).
@defagos defagos converted this from a draft issue Jul 5, 2024
@defagos defagos moved this from ✏️ Draft to 📋 Backlog in Pillarbox Jul 5, 2024
@defagos defagos added the enhancement New feature or request label Jul 5, 2024
@defagos defagos added this to the QoS milestone Jul 5, 2024
@defagos
Copy link
Member Author

defagos commented Jul 13, 2024

Gave a shot at the implementation for fun on 925/avmetrics. Made very significant progress:

  • The API is made of async streams. For insertion into our architecture a custom publisher is required. Fortunately iOS 18 introduces more type information for async streams, which makes such a publisher possible. I implemented such a publisher but I suspect there might be a little issue remaining somewhere, as only the first subscriber to event streams receives their output.
  • It is not possible to have cases in an enum having associated values constrained to some minimal OS version. For this reason, and also because there are many more events delivered, I suggest we keep our events and native events delivered by separate channels.
  • I implemented dedicated publishers. To support event type filtering I had to use parameter packs. That was a fun one. Still, unlike our own implementation, events not caught are lost. I added tasks above to decide which kind of behavior we would like to implement. I guess that some essential events (e.g. content key session, stalls, etc.) could be always captured per current item, cached and delivered on subscription.
  • To have iOS 18 code compile on release and beta versions of Xcode we must use the compiler preprocessor directive. The swift version is namely either 5.10 or 6.0 depending on which version of Swift is adopted.
  • I also think that, even though our events and the native ones can be similar, removing duplicates would be bad. Both APIs are really separate so I guess we can keep redundancies after all.

@defagos defagos moved this from 📋 Backlog to 🚧 In Progress in Pillarbox Jul 14, 2024
@defagos defagos self-assigned this Jul 14, 2024
@defagos
Copy link
Member Author

defagos commented Aug 6, 2024

The single subscriber issue is not related to the AVMetrics API, rather to a known limitation of AsyncStreams.

This issue also affects different streams matching different event types. Only one stream will deliver events, at random if Tasks are started at the same time.

For this reason I think that:

  • We should keep the native metric event streams internal.
  • We should pick which events we are interested in, as the allMetrics() stream contains a lot of noise.
  • We should ensure the stream is used once, either per implementation (single metrics gathering pipeline, like we have today) or make the publisher lazy and shared (but this is probably not a good idea on AVPlayerItem since this requires an associated object and we might run into similar issues as we had with KVO-publishers retaining the observed object).

@defagos
Copy link
Member Author

defagos commented Aug 6, 2024

To avoid issues with marking APIs as available on iOS / tvOS 18+ I would suggest the following:

  • We do not provide AVMetricEvents externally. We always use our MetricEvents.
  • We do not mark events available on 18+ for availability. They will just never be received on lower OS versions.

Since AVMetrics APIs are limited anyway because AsyncStreams can only be consumed once this provides what we expect nicely without making our API unnecessarily complex. Of course AVMetricEvent information is sometimes a bit richer but usually we are interested by a few pieces of information only anyway.

@defagos
Copy link
Member Author

defagos commented Aug 7, 2024

Current state as of beta 5:

  • AVMetrics async sequence cannot be shared but some others like URL.lines can. Not sure if this will change in upcoming betas.
  • The timings retrieved from the content key metric events are currently all zero. The info seems to be there when po-ing the event but is likely not parsed at the moment. Hopefully this will be the case in upcoming betas, otherwise integration of AVMetrics won't be that much useful in the end. The player time and the event date are filled, though (and correct, e.g. if starting playback at some position).
  • The error event stream is degraded in comparison to our error stream, at least for failures. Messages are less explicit (we did some work to have better messages so this likely explains this difference). Moreover playback start errors are reported as recoverable. For fatal errors we likely must stick to our error publishers, for warnings it is yet stll unclear if we can get a cleaner stream than what we have from the error log currently.

@defagos
Copy link
Member Author

defagos commented Aug 12, 2024

Timings from content key metric events still zero on iOS 18 beta 6 😭

@defagos
Copy link
Member Author

defagos commented Aug 21, 2024

Still missing with iOS 18 beta 7.

@defagos
Copy link
Member Author

defagos commented Aug 21, 2024

Radar Missing timing metrics in AVMetricEvents, in particular AVMetricContentKeyRequestEvent filed under FB14875223.

@defagos
Copy link
Member Author

defagos commented Aug 28, 2024

Still missing with iOS 18 beta 8.

@defagos
Copy link
Member Author

defagos commented Sep 6, 2024

I received feedback for FB14875223. This is expected behavior according to Apple, after all there is no URLSession which is officially associated with a AVContentKeySessionDelegate (we introduce our own). From a design perspective I still think that the AVMetrics API is misleading, though.

The recommended approach to obtain metrics is to access the URLSession we use and access its NSURLSessionTaskMetrics. If we really want to extract DRM timings I therefore see three options:

  • We introduce a ContentKeySessionDelegate which we can somehow report timings to the caller. Maybe we have to inject the URLSession or something to return metrics with (e.g. a publisher). But this makes our implementation less universal than it way (we currently support any AVContentKeySessionDelegate).
  • We do as for other metrics, i.e. we assume retrieval starts when the item is current and ends when the corresponding metrics event is received. The date difference provides the timing.
  • We do nothing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: 🚧 In Progress
Development

When branches are created from issues, their pull requests are automatically linked.

1 participant