-
Notifications
You must be signed in to change notification settings - Fork 45
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
Upload Custom Thumbnail for a Lecture #1479
base: dev
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like a good start! This is only a draft but I thought I could leave some input already.
In addition to my thoughts, we need a way during livestreams, to prevent the thumbnail from being overwritten by the automatomatically generated one. My suggestion is, creating a new FileType for costum thumbnails, which is preferred over automatically generated ones.
@@ -50,6 +50,8 @@ func configGinStreamRestRouter(router *gin.Engine, daoWrapper dao.DaoWrapper) { | |||
thumbs.GET(":fid", routes.getThumbs) | |||
thumbs.GET("/live", routes.getLiveThumbs) | |||
thumbs.GET("/vod", routes.getVODThumbs) | |||
thumbs.POST("/customlive", routes.putCustomLiveThumbnail) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need two routes to do the same thing? I'd argue there's no need to distinguish livestreams from vods when uploading thumbnails
@@ -50,6 +50,8 @@ func configGinStreamRestRouter(router *gin.Engine, daoWrapper dao.DaoWrapper) { | |||
thumbs.GET(":fid", routes.getThumbs) | |||
thumbs.GET("/live", routes.getLiveThumbs) | |||
thumbs.GET("/vod", routes.getVODThumbs) | |||
thumbs.POST("/customlive", routes.putCustomLiveThumbnail) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Imo the logical method and path would be something like POST /api/stream/:streamID/thumbnails
thumbs.POST("/customlive", routes.putCustomLiveThumbnail) | |
thumbs.POST("/", routes.putCustomLiveThumbnail) |
return | ||
} | ||
|
||
filename := file.Filename |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove the filename if it's unused
course := tumLiveContext.Course | ||
file, err := c.FormFile("file") | ||
if err != nil { | ||
c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid file"}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can use our RequestError to abort:
_ = c.AbortWithError(http.StatusForbidden, tools.RequestError{
Status: http.StatudBadRequest,
CustomMessage: "Couldn't read file",
Err: err,
})
filesFolder := fmt.Sprintf("%s/%s.%d/%s.%s/files", | ||
tools.Cfg.Paths.Mass, | ||
course.Name, course.Year, | ||
course.Name, course.TeachingTerm) | ||
|
||
path := fmt.Sprintf("%s/%s%s", filesFolder, fileUuid, filepath.Ext(filename)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please use path.Join from the standard library to construct this filepath:
https://pkg.go.dev/path#example-Join
|
||
//tempFilePath := pathprovider.LiveThumbnail(strconv.Itoa(int(streamID))) | ||
if err := c.SaveUploadedFile(file, path); err != nil { | ||
c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to save file"}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use RequestError here too
StreamID: streamID, | ||
Path: path, | ||
Filename: file.Filename, | ||
Type: model.FILETYPE_THUMB_CAM, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
iirc this filetype is for the scrubbar, not the preview. To be sure, use FILETYPE_THUMB_LG_CAM_PRES, which is always elected default
Type: model.FILETYPE_THUMB_CAM, | ||
} | ||
|
||
fileDao := dao.NewFileDao() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need to create a new DAO, just use r.DaoWrapper.FileDao
Motivation and Context
resolves #1155
Description
Currently the thumbnail for the videos is generated automatically. The new Upload Custom Thumbnail button allows the admin users to be able to drag and drop an image file as a custom thumbnail for a lecture. The UI is still a work in progress and the file upload without drag and drop will be supported for better support on other devices such as mobiles.
The current to-dos:
Steps for Testing
Prerequisites:
Screenshots