Dynamic HLS Upload Subfolder #6833
Conversation
|
| String customHeaderStr = getCustomHeaderStr(); | ||
| if(StringUtils.isNotBlank(customHeaderStr)){ | ||
|
|
||
| av_dict_set(options, "headers", customHeaderStr, 0); |
There was a problem hiding this comment.
Please don't use http headers because it makes the implementation more complex.
Just append the generated subfolder path to the end of httpEndpoint. In this way, likely you don't need to change anything in UploadHLSChunk
There was a problem hiding this comment.
I have some concerns about the following approach:
httpEndpoint + getExtendedS3StreamsFolderPath(mainTrackId, streamId, getAppSettings().getS3StreamsFolderPath())
The Issue
I believe we can pass mainTrackId and streamId to UploadHLSChunk this way, but it may not work with variables. By default, getAppSettings().getS3StreamsFolderPath() is set to "streams", and users typically configure this through the s3StreamsFolderPath app setting.
For instance:
`"streams/%m/%s"
If we send an HTTP request like:
http://localhost:5080/WebRTCAppEE/hls-upload/testroom/yunus
It could result in a path like:
streams/%m/%s/testroom/yunus
This happens because UploadHLSChunk constructs the path as:
appSettings.getS3StreamsFolderPath() + File.separator + req.getPathInfo()
This would lead to unexpected behavior if the path contains placeholders like %m and %s. While we could detect the presence of %m and %s in UploadHLSChunk and sanitize them, that would add unnecessary complexity(?). If you approve this i can do it.
Alternative Attempt
Another approach I explored was adding mainTrackId and streamId to the URL via getOutputUrl() as query parameters. However, FFmpeg seems to strip query parameters when they are appended using ?.
Current Solution
Due to the limitations mentioned above, I opted to use headers to pass mainTrackId and streamId. Let me know if there's a better way or if I’m misunderstanding something.
There was a problem hiding this comment.
I understand the problem.
I think all these problems are coming because you use s3StreamsFolderPath. The point in the solution is to creaet a generic solution for subfolder structure. So please create a subFolder settings in the AppSettings and use it.
Fortunately, Broadcast object has also subFolder settings and that parameters passed to the muxers. You'll just need to change StringUtils.isNotBlank(broadcast.getSubFolder) ? broadcast.getSubFolder : appSettings.getSubFolder in the muxer's init methods
There was a problem hiding this comment.
If I add an extra subFolder setting in the app settings, it will result in two settings with similar names, which might confuse users. While this approach simplifies the code, it comes at the cost of making the app less user-friendly/confusing. Had a discusson with @burak-58 about this and he also thought this way. If there are no alternative approaches, I’m awaiting your final approval to proceed with the implementation.
There was a problem hiding this comment.
TBH, If you think two settings make the app less user-friendly/confusing, I can say that they are totally different things.
s3StreamsFolderPath is about S3 root folder where the streams are recorded. subfolder settings are applied to streams not only for S3, it's for local disk recording as well because users are asking for these kinds of settings from time to time.
In our first discussion for this implementation, please correct me if I'm wrong. I remember that we agreed on having another setting for this implementation and adding this setting to the end of httpEndpoint to provide a generic solution. Then you've implemented in another way. I'm wondering what the reason is that you proceed in a different way.
There was a problem hiding this comment.
Hi @lastpeony ,
I suggest to use a simple solution. I describe it in the comment
Hello @mekya |
#6821
todo:
fix commented unittest and extend coverage