Add niconico video Provider #2339
Conversation
…e individual adapters into separate files.
…ate import in album mixin
- Enhanced NiconicoMusicProviderExplorerMixin to fetch recommendations with filtering based on user history, following activities, and like history. - Introduced NiconicoConfig class for managing provider-specific configurations, including recommendation counts and tag filtering. - Implemented TagManager for caching and retrieving video tags asynchronously, with support for deduplication and periodic cleanup. - Updated library mixin to support adding and removing tracks with auto-like functionality. - Improved playlist mixin to handle adding and removing tracks from playlists with error handling. - Added support for fetching and filtering tracks based on required tags in the track mixin. - Created a new configuration file for Niconico provider settings.
…and launch settings
…efactor library mixin
…; update configuration entries for email and sensitive content handling
…r improved track retrieval and error handling
…nfiguration entries for improved content management
…uration and constants for improved tag handling
…ideos for improved track retrieval
- Replaced call_with_throttler with _call_with_throttler in NiconicoVideoAdapter for consistency. - Removed handle_niconico_errors context manager and replaced it with direct logging in various mixins. - Introduced log_verbose and log_verbose_operation functions for improved logging. - Updated methods across mixins to utilize new logging functions and streamline error handling. - Refactored tag management to improve caching and fetching logic.
…enhance error handling, and streamline stream format processing
… to reduce database load
Replace StreamType.HTTP with CUSTOM for optimized seeking support. Key improvements: - Parse m3u8 once during stream conversion (not per seek) - Generate dynamic m3u8 starting from seek segment - Separate responsibilities: fetch → parse → generate → stream - Use VERBOSE logging for seek operations - Organize helpers into directory structure
…rride in TrackMixin
…ation - Add comprehensive explanation in stream.py for why CUSTOM is used - Document two-stage seek optimization (segment selection + input-side -ss) - Explain input-side -ss limitation (can't cross segment boundaries) - Add stage-by-stage comments in get_audio_stream and create_stream_context - Clarify performance trade-off vs output-side -ss
Major changes: - Rename NicovideoHLSProcessor → HLSSeekOptimizer for clarity - Extract HLS data models to separate hls_models.py file - Standardize m3u8 → playlist terminology throughout codebase - Add HLS prefix to public API names to avoid confusion with track playlists - Introduce DOMAND_BID_COOKIE_NAME constant for authentication - Update method names for better semantic clarity: * _get_stream_data → _prepare_conversion_data * convert_by_stream_data → convert_from_conversion_data * _fetch_hls_m3u8_text → _fetch_media_playlist_text * _serve_m3u8 → _serve_hls_playlist File structure: - hls_processor.py → hls_seek_optimizer.py (renamed) - hls_models.py (new): ParsedHLSPlaylist, HLSSegment All tests and fixtures updated accordingly.
- Simplify TypeToConverterMappingRegistry (remove 3 unused methods) - Extract all stabilization logic into FieldStabilizer class - Add helper method to ConverterTestRunner for clarity - Document FixtureProcessorProtocol design intent Reduces complexity while maintaining type safety and auto-update features
Renamed classes for clarity: - TypeToConverterMapping → APIResponseConverterMapping - FixtureManager → FixtureLoader - FixtureDataGenerators → APIFixtureCollector - FixtureDataSaver → FixtureSaver - PathToTypeMapper → FixtureTypeMappingCollector + FixtureTypeMappingFileGenerator Changes: - Updated method names: generate_* → collect_* in APIFixtureCollector - Fixed type completion by changing list to tuple for API_RESPONSE_CONVERTER_MAPPINGS - Removed re-export pattern from fixtures/__init__.py - Added comprehensive documentation to test_converters.py Benefits: clearer responsibilities, better type inference, improved maintainability
Yes, this has come up a few times for consistency reasons people seem to always prefer the album image. Can we flip this question, why is this provider an exception and should the track thumb be preferred ? |
|
In this provider, Niconico's "Series" feature is mapped to albums. Why Track Thumbnails Should Be Preferred for Niconico1. Series/Album Structure Characteristics:
2. Track Thumbnails Are Essential Identity:
3. User Experience Impact:
Alternative ApproachIf modifying the core behavior is not desirable, I would like to prevent this provider from using album images entirely to maintain consistency with the Niconico platform's UX expectations. |
For now, I can not fully oversee the implications of modifying the core behavior but I will change it anyways - we can use the beta period to collect user feedback and if some unexpected side effect is reported, we will need to find a way to specify the behavior fine grained per provider. |
There was a problem hiding this comment.
please remove this from the PR, if you need to do specific CI tasks and testing of your code for translating api to python models, that needs to be in a dedicated repository for your client library.
There was a problem hiding this comment.
Thanks for the feedback. Removed the CI workflow and all fixture generation infrastructure from this PR.
Only test execution code and static fixtures remain.
There was a problem hiding this comment.
please limit your PR to you provider only.
Adding this quick launch command does however make a lot of sense, it just belongs in a dedicated PR
There was a problem hiding this comment.
Removed the VS Code launch configuration changes.
| Note: While MusicAssistant's get_provider_item automatically handles cache retrieval | ||
| and storage, this helper function is needed for explicit cache updates (e.g., when | ||
| adding album information to tracks) since MusicAssistant doesn't provide a dedicated | ||
| cache-only update function. | ||
| """ | ||
| cache_key = f"track.{track.item_id}" | ||
| await provider.mass.cache.set( | ||
| cache_key, | ||
| track.to_dict(), | ||
| provider=provider.lookup_key, | ||
| ) |
There was a problem hiding this comment.
Note that we recently did a bunch of changes to caching, basically we removed "automagic" caching by default from the core controllers making it a responsibility of the providers - which know better what and how long to cache.
Also we added support for a cache category to more fine grained split up the results in the cache (and it will give a bit of s speed boost due the indexes)
There was a problem hiding this comment.
Migrated to the new @use_cache decorator system.
Applied cache durations based on SoundCloud patterns:
- get_artist/track/playlist: 14 days
- search: 3 hours
| async for track in self.mass.music.tracks.iter_library_items( | ||
| provider=self.instance_id, | ||
| ): | ||
| for artist in track.artists: |
There was a problem hiding this comment.
not sure I like this mixup of iterating the tracks controller from within a provider.
You should only return in-library or favorited artists here, nothing more, nothing less. If your provider does not have a concept of library artists, just leave out the feature to fallback to the library database of collecting this for you.
There was a problem hiding this comment.
Fixed by removing the tracks controller iteration. The method now only returns followed artists from Niconico's native following system, which represents the actual in-library artists for this provider.
Also removed get_library_tracks and get_library_albums methods that had similar issues, along with their corresponding ProviderFeature flags.
| following_artists = await self.service_manager.user.get_own_followings() | ||
| for artist in following_artists: | ||
| yield artist |
There was a problem hiding this comment.
yeah, this is the only part you should return, scrap the part above
There was a problem hiding this comment.
tbh I think all the fixture creating stuff needs to exist in a dedicated (client library) repository to split up responsibilities.
There was a problem hiding this comment.
Created dedicated repository: music-assistant-nicovideo-fixtures
Separation:
- Fixture generation repo: API collection, processing, type mapping generation
- This PR: Static fixtures + test execution only
Update process: Generate fixtures in dedicated repo → copy to tests/providers/nicovideo/fixture_data
This keeps generation logic and credentials out of the main codebase.
- Move fixture generation to separate repository (music_assistant_nicovideo_fixtures) - Restructure fixtures: use fixture_data/ for generated data - Update fixture loading to use new structure with fixture_type_mappings - Remove embedded fixture generation scripts from tests - Add comprehensive README for test directory - Update VS Code tasks for easier fixture updates - Delete obsolete GitHub workflow for fixture generation
Shi-553
force-pushed
the
add-niconico-provider
branch
from
aa4d383 to
e9da790
Compare
- Apply @use_cache decorator to get_* methods for improved performance - Remove manual cache handling (cache_track helper and complex album info updates) - Adjust cache durations based on SoundCloud patterns: - get_artist: 14 days (was 30 days) - get_track: 14 days (was 7 days) - get_playlist: 14 days (was 30 days) - get_playlist_tracks: 3 hours (unchanged) - search: 3 hours (was 1 hour) - get_album methods: 7 days (unchanged) - Simplify code by leveraging Music Assistant's standard caching system - Fix imports and add proper noqa comments for @use_cache usage
Remove the 'Music Assistant: Snapshot Update' launch configuration that was added in previous commits. As requested in PR review, this change belongs in a separate PR focused on development tools rather than the nicovideo provider implementation.
Address PR feedback by removing methods that iterate other controllers: - Remove get_library_tracks and get_library_albums methods - Remove library_add_for_mixin and library_remove_for_mixin methods - Remove get_library_artists iteration over tracks controller - Remove corresponding ProviderFeature flags from SUPPORTED_FEATURES - Fix imports and TYPE_CHECKING blocks The provider now only handles its native concepts (followed artists, mylists, series) without inappropriately accessing other controllers. This aligns with Music Assistant's architecture where providers should only return in-library or favorited items from their own domain.
Remove category=1 parameter from @use_cache decorator in get_track method to maintain consistency with other cached methods that use default category 0. This aligns with the pattern used across other providers and removes unnecessary complexity.
…time errors - Move Album, Artist, Track imports from TYPE_CHECKING to regular imports in artist.py - Add noqa: TC002 comments for @use_cache decorator compatibility - Format media_items imports in track.py consistently - Fixes NameError: name 'Artist' is not defined when using @use_cache decorator - Aligns with other Music Assistant provider patterns
- Call get_track directly instead of get_provider_item for simplicity - Remove unnecessary semaphore (throttler already handles rate limiting) - Fix throttler settings: rate_limit=5, period=1 (5 req/sec) for high priority - Fix low priority throttler: period=1 for consistency
3 participants
This PR adds a new provider that enables searching, playback, and library integration for music content on niconico video (nicovideo.jp) within Music Assistant.
Highlights
Authentication
user_session(cookie); on failure, fall back to Email/Password (+MFA) [auth.py]Dependencies
Tests and Fixtures
Known Issues