Skip to content
New issue

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

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

Already on GitHub? # to your account

Videos: Convert URL before putting result into cache #4850

Merged
merged 1 commit into from
Oct 8, 2024

Conversation

SamantazFox
Copy link
Member

Closes #4837

@SamantazFox SamantazFox requested a review from a team as a code owner August 15, 2024 16:11
@SamantazFox SamantazFox requested review from unixfox and removed request for a team August 15, 2024 16:11
@SamantazFox SamantazFox force-pushed the url-convert-before-cache branch from ed121e3 to dc5e9c2 Compare August 16, 2024 13:25
@syeopite syeopite self-requested a review August 17, 2024 03:02
Comment on lines 139 to 149
# This part is complex just because we can't directly edit inside a JSON::Any
# object, so we have to unpack everything, just to repack it afterwards.
if streaming_data = player_response["streamingData"]?.try &.as_h
new_streaming_data = {} of String => JSON::Any

streaming_data.each do |key, value|
if key == "formats" || key == "adaptiveFormats"
new_formats = [] of JSON::Any

value.as_a.map(&.as_h).each do |fmt|
fmt["url"] = JSON::Any.new(convert_url(fmt))
new_formats << JSON::Any.new(fmt)
end

new_streaming_data[key] = JSON::Any.new(new_formats)
else
new_streaming_data[key] = value
end
end

params["streamingData"] = JSON::Any.new(new_streaming_data)
end

Choose a reason for hiding this comment

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

suggestion: It's entirely possible to mutate data in a JSON::Any. So I think this can be simplified quite a lot to something like this:

  if streaming_data = player_response["streamingData"]?
    %w[formats adaptiveFormats].each do |key|
      streaming_data.as_h[key]?.try &.as_a.each do |format|
        format.as_h["url"] = JSON::Any.new(convert_url(format))
      end
    end
  end

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks a lot! Now that seems obvious ^^

@SamantazFox SamantazFox force-pushed the url-convert-before-cache branch from dc5e9c2 to b2133c6 Compare August 24, 2024 16:02
@SamantazFox SamantazFox added the in-testing This feature has been deployed and is being tested label Aug 24, 2024
@SamantazFox SamantazFox added ready and removed in-testing This feature has been deployed and is being tested labels Oct 6, 2024
@SamantazFox SamantazFox merged commit 3cfcc16 into iv-org:master Oct 8, 2024
7 of 8 checks passed
@SamantazFox SamantazFox deleted the url-convert-before-cache branch October 8, 2024 16:38
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Don't cache videoplayback if player has been updated
2 participants